Received-SPF: pass (google.com: domain of linux-ext4+bounces-2036-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 11 Apr 2024 10:55:59 -0400
From: "Theodore Ts'o" <tytso@mit.edu>
To: Jan Kara <jack@suse.cz>
Cc: "yebin (H)" <yebin10@huawei.com>, adilger.kernel@dilger.ca,
        linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] jbd2: avoid mount failed when commit block is partial
 submitted
Message-ID: <20240411145559.GB187181@mit.edu>
References: <20240402090951.527619-1-yebin10@huawei.com>
 <20240402134240.5he4mxei3nvzolb3@quack3>
 <20240403033742.GE1189142@mit.edu>
 <20240403101122.rmffivvvf4a33qis@quack3>
 <6611F8D5.3030403@huawei.com>
 <20240411133718.tq74yorf6odpla4r@quack3>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240411133718.tq74yorf6odpla4r@quack3>

On Thu, Apr 11, 2024 at 03:37:18PM +0200, Jan Kara wrote:
> > The vendor
> > has confirmed that only 512-byte atomicity can be ensured in the firmware.
> > Although the valid data is only 60 bytes, the entire commit block is used
> > for calculating
> > the checksum.
> > jbd2_commit_block_csum_verify:
> > ...
> > calculated = jbd2_chksum(j, j->j_csum_seed, buf, j->j_blocksize);
> > ...
> 
> Ah, indeed. This is the bit I've missed. Thanks for explanation! Still I
> think trying to somehow automatically deal with wrong commit block checksum
> is too dangerous because it can result in fs corruption in some (unlikely)
> cases. OTOH I understand journal replay failure after a power fail isn't
> great either so we need to think how to fix this...

Unfortunately, the only fix I can think of would require changing how
we do the checksum to only include the portion of the jbd2 block which
contains valid data, per the header field.  This would be a format
change which means that if a new kernel writes the new jbd2 format
(using a journal incompat flag, or a new checksum type), older kernels
and older versions of e2fsprogs wouldn't be able to validate the
journal.  So rollout of the fix would have to be carefully managed.

					- Ted