[PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb

[PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Copying the file system while it is mounted as read-only results in
a mount failure:
[~]# mkfs.ext4 -F /dev/sdc
[~]# mount /dev/sdc -o ro /mnt/test
[~]# dd if=/dev/sdc of=/dev/sda bs=1M
[~]# mount /dev/sda /mnt/test1
[ 1094.849826] JBD2: journal checksum error
[ 1094.850927] EXT4-fs (sda): Could not load journal inode
mount: mount /dev/sda on /mnt/test1 failed: Bad message

Above issue may happen as follows:
ext4_fill_super
 set_journal_csum_feature_set(sb)
  if (ext4_has_metadata_csum(sb))
   incompat = JBD2_FEATURE_INCOMPAT_CSUM_V3;
  if (test_opt(sb, JOURNAL_CHECKSUM)
   jbd2_journal_set_features(sbi->s_journal, compat, 0, incompat);
    lock_buffer(journal->j_sb_buffer);
    sb->s_feature_incompat  |= cpu_to_be32(incompat);
    //The data in the journal sb was modified, but the checksum was not
      updated, so the data remaining in memory has a mismatch between the
      data and the checksum.
    unlock_buffer(journal->j_sb_buffer);

In this case, the journal sb copied over is in a state where the checksum
and data are inconsistent, so mounting fails.
To solve the above issue, update the checksum in memory after modifying
the journal sb.

Fixes: 4fd5ea43bc11 ("jbd2: checksum journal superblock")
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/jbd2/journal.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index d480b94117cd..5b6e8c1a5e6a 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2349,6 +2349,8 @@ int jbd2_journal_set_features(journal_t *journal, unsigned long compat,
 	sb->s_feature_compat    |= cpu_to_be32(compat);
 	sb->s_feature_ro_compat |= cpu_to_be32(ro);
 	sb->s_feature_incompat  |= cpu_to_be32(incompat);
+	if (jbd2_journal_has_csum_v2or3(journal))
+		sb->s_checksum = jbd2_superblock_csum(sb);
 	unlock_buffer(journal->j_sb_buffer);
 	jbd2_journal_init_transaction_limits(journal);
 
@@ -2378,9 +2380,13 @@ void jbd2_journal_clear_features(journal_t *journal, unsigned long compat,
 
 	sb = journal->j_superblock;
 
+	lock_buffer(journal->j_sb_buffer);
 	sb->s_feature_compat    &= ~cpu_to_be32(compat);
 	sb->s_feature_ro_compat &= ~cpu_to_be32(ro);
 	sb->s_feature_incompat  &= ~cpu_to_be32(incompat);
+	if (jbd2_journal_has_csum_v2or3(journal))
+		sb->s_checksum = jbd2_superblock_csum(sb);
+	unlock_buffer(journal->j_sb_buffer);
 	jbd2_journal_init_transaction_limits(journal);
 }
 EXPORT_SYMBOL(jbd2_journal_clear_features);
-- 
2.34.1

Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb

[Zhang Yi]

Hi Bin!

On 10/28/2025 2:47 PM, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> Copying the file system while it is mounted as read-only results in
> a mount failure:
> [~]# mkfs.ext4 -F /dev/sdc
> [~]# mount /dev/sdc -o ro /mnt/test
> [~]# dd if=/dev/sdc of=/dev/sda bs=1M
> [~]# mount /dev/sda /mnt/test1
> [ 1094.849826] JBD2: journal checksum error
> [ 1094.850927] EXT4-fs (sda): Could not load journal inode
> mount: mount /dev/sda on /mnt/test1 failed: Bad message
> 

I suppose we need to explain why we have this particular use case.

> Above issue may happen as follows:
> ext4_fill_super
>  set_journal_csum_feature_set(sb)
>   if (ext4_has_metadata_csum(sb))
>    incompat = JBD2_FEATURE_INCOMPAT_CSUM_V3;
>   if (test_opt(sb, JOURNAL_CHECKSUM)
>    jbd2_journal_set_features(sbi->s_journal, compat, 0, incompat);
>     lock_buffer(journal->j_sb_buffer);
>     sb->s_feature_incompat  |= cpu_to_be32(incompat);
>     //The data in the journal sb was modified, but the checksum was not
>       updated, so the data remaining in memory has a mismatch between the
>       data and the checksum.
>     unlock_buffer(journal->j_sb_buffer);
> 
> In this case, the journal sb copied over is in a state where the checksum
> and data are inconsistent, so mounting fails.
> To solve the above issue, update the checksum in memory after modifying
> the journal sb.

What about checking sb_rdonly(sb) before setting the journal feature, and
skipping the setting if the filesystem is mounted as read-only?

Regards,
Yi.

> 
> Fixes: 4fd5ea43bc11 ("jbd2: checksum journal superblock")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/jbd2/journal.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index d480b94117cd..5b6e8c1a5e6a 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -2349,6 +2349,8 @@ int jbd2_journal_set_features(journal_t *journal, unsigned long compat,
>  	sb->s_feature_compat    |= cpu_to_be32(compat);
>  	sb->s_feature_ro_compat |= cpu_to_be32(ro);
>  	sb->s_feature_incompat  |= cpu_to_be32(incompat);
> +	if (jbd2_journal_has_csum_v2or3(journal))
> +		sb->s_checksum = jbd2_superblock_csum(sb);
>  	unlock_buffer(journal->j_sb_buffer);
>  	jbd2_journal_init_transaction_limits(journal);
>  
> @@ -2378,9 +2380,13 @@ void jbd2_journal_clear_features(journal_t *journal, unsigned long compat,
>  
>  	sb = journal->j_superblock;
>  
> +	lock_buffer(journal->j_sb_buffer);
>  	sb->s_feature_compat    &= ~cpu_to_be32(compat);
>  	sb->s_feature_ro_compat &= ~cpu_to_be32(ro);
>  	sb->s_feature_incompat  &= ~cpu_to_be32(incompat);
> +	if (jbd2_journal_has_csum_v2or3(journal))
> +		sb->s_checksum = jbd2_superblock_csum(sb);
> +	unlock_buffer(journal->j_sb_buffer);
>  	jbd2_journal_init_transaction_limits(journal);
>  }
>  EXPORT_SYMBOL(jbd2_journal_clear_features);

Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb

[yebin]

On 2025/10/29 16:45, Zhang Yi wrote:
> Hi Bin!
>
> On 10/28/2025 2:47 PM, Ye Bin wrote:
>> From: Ye Bin <yebin10@huawei.com>
>>
>> Copying the file system while it is mounted as read-only results in
>> a mount failure:
>> [~]# mkfs.ext4 -F /dev/sdc
>> [~]# mount /dev/sdc -o ro /mnt/test
>> [~]# dd if=/dev/sdc of=/dev/sda bs=1M
>> [~]# mount /dev/sda /mnt/test1
>> [ 1094.849826] JBD2: journal checksum error
>> [ 1094.850927] EXT4-fs (sda): Could not load journal inode
>> mount: mount /dev/sda on /mnt/test1 failed: Bad message
>>
>
> I suppose we need to explain why we have this particular use case.
>
The process described above is just an abstracted way I came up with to 
reproduce the issue. In the actual scenario, the file system was mounted 
read-only and then copied while it was still mounted. It was found that 
the mount operation failed. The user intended to verify the data or use 
it as a backup, and this action was performed during a version upgrade.

>> Above issue may happen as follows:
>> ext4_fill_super
>>   set_journal_csum_feature_set(sb)
>>    if (ext4_has_metadata_csum(sb))
>>     incompat = JBD2_FEATURE_INCOMPAT_CSUM_V3;
>>    if (test_opt(sb, JOURNAL_CHECKSUM)
>>     jbd2_journal_set_features(sbi->s_journal, compat, 0, incompat);
>>      lock_buffer(journal->j_sb_buffer);
>>      sb->s_feature_incompat  |= cpu_to_be32(incompat);
>>      //The data in the journal sb was modified, but the checksum was not
>>        updated, so the data remaining in memory has a mismatch between the
>>        data and the checksum.
>>      unlock_buffer(journal->j_sb_buffer);
>>
>> In this case, the journal sb copied over is in a state where the checksum
>> and data are inconsistent, so mounting fails.
>> To solve the above issue, update the checksum in memory after modifying
>> the journal sb.
>
> What about checking sb_rdonly(sb) before setting the journal feature, and
> skipping the setting if the filesystem is mounted as read-only?

Yes, I've thought about doing that too, but that would require us to set 
the journal's features again when remounting as rw.

>
> Regards,
> Yi.
>
>>
>> Fixes: 4fd5ea43bc11 ("jbd2: checksum journal superblock")
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>>   fs/jbd2/journal.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index d480b94117cd..5b6e8c1a5e6a 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -2349,6 +2349,8 @@ int jbd2_journal_set_features(journal_t *journal, unsigned long compat,
>>   	sb->s_feature_compat    |= cpu_to_be32(compat);
>>   	sb->s_feature_ro_compat |= cpu_to_be32(ro);
>>   	sb->s_feature_incompat  |= cpu_to_be32(incompat);
>> +	if (jbd2_journal_has_csum_v2or3(journal))
>> +		sb->s_checksum = jbd2_superblock_csum(sb);
>>   	unlock_buffer(journal->j_sb_buffer);
>>   	jbd2_journal_init_transaction_limits(journal);
>>
>> @@ -2378,9 +2380,13 @@ void jbd2_journal_clear_features(journal_t *journal, unsigned long compat,
>>
>>   	sb = journal->j_superblock;
>>
>> +	lock_buffer(journal->j_sb_buffer);
>>   	sb->s_feature_compat    &= ~cpu_to_be32(compat);
>>   	sb->s_feature_ro_compat &= ~cpu_to_be32(ro);
>>   	sb->s_feature_incompat  &= ~cpu_to_be32(incompat);
>> +	if (jbd2_journal_has_csum_v2or3(journal))
>> +		sb->s_checksum = jbd2_superblock_csum(sb);
>> +	unlock_buffer(journal->j_sb_buffer);
>>   	jbd2_journal_init_transaction_limits(journal);
>>   }
>>   EXPORT_SYMBOL(jbd2_journal_clear_features);
>

Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb

[Darrick J. Wong]

On Tue, Oct 28, 2025 at 02:47:28PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> Copying the file system while it is mounted as read-only results in
> a mount failure:
> [~]# mkfs.ext4 -F /dev/sdc
> [~]# mount /dev/sdc -o ro /mnt/test
> [~]# dd if=/dev/sdc of=/dev/sda bs=1M
> [~]# mount /dev/sda /mnt/test1
> [ 1094.849826] JBD2: journal checksum error
> [ 1094.850927] EXT4-fs (sda): Could not load journal inode
> mount: mount /dev/sda on /mnt/test1 failed: Bad message

I was about to say "Well don't do that, freeze the fs first..."

> Above issue may happen as follows:
> ext4_fill_super
>  set_journal_csum_feature_set(sb)
>   if (ext4_has_metadata_csum(sb))
>    incompat = JBD2_FEATURE_INCOMPAT_CSUM_V3;
>   if (test_opt(sb, JOURNAL_CHECKSUM)
>    jbd2_journal_set_features(sbi->s_journal, compat, 0, incompat);
>     lock_buffer(journal->j_sb_buffer);
>     sb->s_feature_incompat  |= cpu_to_be32(incompat);
>     //The data in the journal sb was modified, but the checksum was not
>       updated, so the data remaining in memory has a mismatch between the
>       data and the checksum.
>     unlock_buffer(journal->j_sb_buffer);
> 
> In this case, the journal sb copied over is in a state where the checksum
> and data are inconsistent, so mounting fails.
> To solve the above issue, update the checksum in memory after modifying
> the journal sb.

...but I think the actual change is correct because (a) we shouldn't
unlock the bh with an incorrect checksum because userspace can see that;
and (b) if the bh ever gets marked dirty, then writeback can push the
inconsistent buffer to disk at any time.

I think it's the case that j_sb_buffer is only ever written out
explicitly with submit_bh rather than going through the dirty -> flush
machinery, but I guess syzbot could read and write the same value from
userspace to dirty the buffer and flush it out while racing to shut down
the journal, and now the ondisk journal is inconsistent.

Anyway, the "set csum before unlock_buffer" paradigm is all over the
ext4 code so

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> Fixes: 4fd5ea43bc11 ("jbd2: checksum journal superblock")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/jbd2/journal.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index d480b94117cd..5b6e8c1a5e6a 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -2349,6 +2349,8 @@ int jbd2_journal_set_features(journal_t *journal, unsigned long compat,
>  	sb->s_feature_compat    |= cpu_to_be32(compat);
>  	sb->s_feature_ro_compat |= cpu_to_be32(ro);
>  	sb->s_feature_incompat  |= cpu_to_be32(incompat);
> +	if (jbd2_journal_has_csum_v2or3(journal))
> +		sb->s_checksum = jbd2_superblock_csum(sb);
>  	unlock_buffer(journal->j_sb_buffer);
>  	jbd2_journal_init_transaction_limits(journal);
>  
> @@ -2378,9 +2380,13 @@ void jbd2_journal_clear_features(journal_t *journal, unsigned long compat,
>  
>  	sb = journal->j_superblock;
>  
> +	lock_buffer(journal->j_sb_buffer);
>  	sb->s_feature_compat    &= ~cpu_to_be32(compat);
>  	sb->s_feature_ro_compat &= ~cpu_to_be32(ro);
>  	sb->s_feature_incompat  &= ~cpu_to_be32(incompat);
> +	if (jbd2_journal_has_csum_v2or3(journal))
> +		sb->s_checksum = jbd2_superblock_csum(sb);
> +	unlock_buffer(journal->j_sb_buffer);
>  	jbd2_journal_init_transaction_limits(journal);
>  }
>  EXPORT_SYMBOL(jbd2_journal_clear_features);
> -- 
> 2.34.1
> 
> 

Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb

[Zhang Yi]

On 10/29/2025 10:55 PM, Darrick J. Wong wrote:
> On Tue, Oct 28, 2025 at 02:47:28PM +0800, Ye Bin wrote:
>> From: Ye Bin <yebin10@huawei.com>
>>
>> Copying the file system while it is mounted as read-only results in
>> a mount failure:
>> [~]# mkfs.ext4 -F /dev/sdc
>> [~]# mount /dev/sdc -o ro /mnt/test
>> [~]# dd if=/dev/sdc of=/dev/sda bs=1M
>> [~]# mount /dev/sda /mnt/test1
>> [ 1094.849826] JBD2: journal checksum error
>> [ 1094.850927] EXT4-fs (sda): Could not load journal inode
>> mount: mount /dev/sda on /mnt/test1 failed: Bad message
> 
> I was about to say "Well don't do that, freeze the fs first..."
> 

Yeah, this step is indeed necessary! However, it does not work for
the current case because there is a check for read-only mode in
freeze_super(), which assumes that no modifications to the file
system will occur in read-only mode, thus skipping the freezing of
the file system.

Thanks,
Yi.

>> Above issue may happen as follows:
>> ext4_fill_super
>>  set_journal_csum_feature_set(sb)
>>   if (ext4_has_metadata_csum(sb))
>>    incompat = JBD2_FEATURE_INCOMPAT_CSUM_V3;
>>   if (test_opt(sb, JOURNAL_CHECKSUM)
>>    jbd2_journal_set_features(sbi->s_journal, compat, 0, incompat);
>>     lock_buffer(journal->j_sb_buffer);
>>     sb->s_feature_incompat  |= cpu_to_be32(incompat);
>>     //The data in the journal sb was modified, but the checksum was not
>>       updated, so the data remaining in memory has a mismatch between the
>>       data and the checksum.
>>     unlock_buffer(journal->j_sb_buffer);
>>
>> In this case, the journal sb copied over is in a state where the checksum
>> and data are inconsistent, so mounting fails.
>> To solve the above issue, update the checksum in memory after modifying
>> the journal sb.
> 
> ...but I think the actual change is correct because (a) we shouldn't
> unlock the bh with an incorrect checksum because userspace can see that;
> and (b) if the bh ever gets marked dirty, then writeback can push the
> inconsistent buffer to disk at any time.
> 
> I think it's the case that j_sb_buffer is only ever written out
> explicitly with submit_bh rather than going through the dirty -> flush
> machinery, but I guess syzbot could read and write the same value from
> userspace to dirty the buffer and flush it out while racing to shut down
> the journal, and now the ondisk journal is inconsistent.
> 
> Anyway, the "set csum before unlock_buffer" paradigm is all over the
> ext4 code so
> 
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> --D
> 
>> Fixes: 4fd5ea43bc11 ("jbd2: checksum journal superblock")
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>>  fs/jbd2/journal.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index d480b94117cd..5b6e8c1a5e6a 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -2349,6 +2349,8 @@ int jbd2_journal_set_features(journal_t *journal, unsigned long compat,
>>  	sb->s_feature_compat    |= cpu_to_be32(compat);
>>  	sb->s_feature_ro_compat |= cpu_to_be32(ro);
>>  	sb->s_feature_incompat  |= cpu_to_be32(incompat);
>> +	if (jbd2_journal_has_csum_v2or3(journal))
>> +		sb->s_checksum = jbd2_superblock_csum(sb);
>>  	unlock_buffer(journal->j_sb_buffer);
>>  	jbd2_journal_init_transaction_limits(journal);
>>  
>> @@ -2378,9 +2380,13 @@ void jbd2_journal_clear_features(journal_t *journal, unsigned long compat,
>>  
>>  	sb = journal->j_superblock;
>>  
>> +	lock_buffer(journal->j_sb_buffer);
>>  	sb->s_feature_compat    &= ~cpu_to_be32(compat);
>>  	sb->s_feature_ro_compat &= ~cpu_to_be32(ro);
>>  	sb->s_feature_incompat  &= ~cpu_to_be32(incompat);
>> +	if (jbd2_journal_has_csum_v2or3(journal))
>> +		sb->s_checksum = jbd2_superblock_csum(sb);
>> +	unlock_buffer(journal->j_sb_buffer);
>>  	jbd2_journal_init_transaction_limits(journal);
>>  }
>>  EXPORT_SYMBOL(jbd2_journal_clear_features);
>> -- 
>> 2.34.1
>>
>>
> 

Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb

[Jan Kara]

On Tue 28-10-25 14:47:28, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> Copying the file system while it is mounted as read-only results in
> a mount failure:
> [~]# mkfs.ext4 -F /dev/sdc
> [~]# mount /dev/sdc -o ro /mnt/test
> [~]# dd if=/dev/sdc of=/dev/sda bs=1M
> [~]# mount /dev/sda /mnt/test1
> [ 1094.849826] JBD2: journal checksum error
> [ 1094.850927] EXT4-fs (sda): Could not load journal inode
> mount: mount /dev/sda on /mnt/test1 failed: Bad message
> 
> Above issue may happen as follows:
> ext4_fill_super
>  set_journal_csum_feature_set(sb)
>   if (ext4_has_metadata_csum(sb))
>    incompat = JBD2_FEATURE_INCOMPAT_CSUM_V3;
>   if (test_opt(sb, JOURNAL_CHECKSUM)
>    jbd2_journal_set_features(sbi->s_journal, compat, 0, incompat);
>     lock_buffer(journal->j_sb_buffer);
>     sb->s_feature_incompat  |= cpu_to_be32(incompat);
>     //The data in the journal sb was modified, but the checksum was not
>       updated, so the data remaining in memory has a mismatch between the
>       data and the checksum.
>     unlock_buffer(journal->j_sb_buffer);
> 
> In this case, the journal sb copied over is in a state where the checksum
> and data are inconsistent, so mounting fails.
> To solve the above issue, update the checksum in memory after modifying
> the journal sb.
> 
> Fixes: 4fd5ea43bc11 ("jbd2: checksum journal superblock")
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Looks good but please add a short comment before the checksum update
explaining why jbd2_superblock_csum() call in jbd2_write_superblock() isn't
enough. Something like:

	/*
	 * Update the checksum now so that it is valid even for read-only
	 * filesystems where jbd2_write_superblock() doesn't get called.
	 */

Otherwise feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/journal.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index d480b94117cd..5b6e8c1a5e6a 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -2349,6 +2349,8 @@ int jbd2_journal_set_features(journal_t *journal, unsigned long compat,
>  	sb->s_feature_compat    |= cpu_to_be32(compat);
>  	sb->s_feature_ro_compat |= cpu_to_be32(ro);
>  	sb->s_feature_incompat  |= cpu_to_be32(incompat);
> +	if (jbd2_journal_has_csum_v2or3(journal))
> +		sb->s_checksum = jbd2_superblock_csum(sb);
>  	unlock_buffer(journal->j_sb_buffer);
>  	jbd2_journal_init_transaction_limits(journal);
>  
> @@ -2378,9 +2380,13 @@ void jbd2_journal_clear_features(journal_t *journal, unsigned long compat,
>  
>  	sb = journal->j_superblock;
>  
> +	lock_buffer(journal->j_sb_buffer);
>  	sb->s_feature_compat    &= ~cpu_to_be32(compat);
>  	sb->s_feature_ro_compat &= ~cpu_to_be32(ro);
>  	sb->s_feature_incompat  &= ~cpu_to_be32(incompat);
> +	if (jbd2_journal_has_csum_v2or3(journal))
> +		sb->s_checksum = jbd2_superblock_csum(sb);
> +	unlock_buffer(journal->j_sb_buffer);
>  	jbd2_journal_init_transaction_limits(journal);
>  }
>  EXPORT_SYMBOL(jbd2_journal_clear_features);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb

[yebin]

On 2025/10/30 18:46, Jan Kara wrote:
> On Tue 28-10-25 14:47:28, Ye Bin wrote:
>> From: Ye Bin <yebin10@huawei.com>
>>
>> Copying the file system while it is mounted as read-only results in
>> a mount failure:
>> [~]# mkfs.ext4 -F /dev/sdc
>> [~]# mount /dev/sdc -o ro /mnt/test
>> [~]# dd if=/dev/sdc of=/dev/sda bs=1M
>> [~]# mount /dev/sda /mnt/test1
>> [ 1094.849826] JBD2: journal checksum error
>> [ 1094.850927] EXT4-fs (sda): Could not load journal inode
>> mount: mount /dev/sda on /mnt/test1 failed: Bad message
>>
>> Above issue may happen as follows:
>> ext4_fill_super
>>   set_journal_csum_feature_set(sb)
>>    if (ext4_has_metadata_csum(sb))
>>     incompat = JBD2_FEATURE_INCOMPAT_CSUM_V3;
>>    if (test_opt(sb, JOURNAL_CHECKSUM)
>>     jbd2_journal_set_features(sbi->s_journal, compat, 0, incompat);
>>      lock_buffer(journal->j_sb_buffer);
>>      sb->s_feature_incompat  |= cpu_to_be32(incompat);
>>      //The data in the journal sb was modified, but the checksum was not
>>        updated, so the data remaining in memory has a mismatch between the
>>        data and the checksum.
>>      unlock_buffer(journal->j_sb_buffer);
>>
>> In this case, the journal sb copied over is in a state where the checksum
>> and data are inconsistent, so mounting fails.
>> To solve the above issue, update the checksum in memory after modifying
>> the journal sb.
>>
>> Fixes: 4fd5ea43bc11 ("jbd2: checksum journal superblock")
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>
> Looks good but please add a short comment before the checksum update
> explaining why jbd2_superblock_csum() call in jbd2_write_superblock() isn't
> enough. Something like:
>
> 	/*
> 	 * Update the checksum now so that it is valid even for read-only
> 	 * filesystems where jbd2_write_superblock() doesn't get called.
> 	 */
>
Thank you for your suggestion. I will resend a new version.

> Otherwise feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> 								Honza
>
>> ---
>>   fs/jbd2/journal.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index d480b94117cd..5b6e8c1a5e6a 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -2349,6 +2349,8 @@ int jbd2_journal_set_features(journal_t *journal, unsigned long compat,
>>   	sb->s_feature_compat    |= cpu_to_be32(compat);
>>   	sb->s_feature_ro_compat |= cpu_to_be32(ro);
>>   	sb->s_feature_incompat  |= cpu_to_be32(incompat);
>> +	if (jbd2_journal_has_csum_v2or3(journal))
>> +		sb->s_checksum = jbd2_superblock_csum(sb);
>>   	unlock_buffer(journal->j_sb_buffer);
>>   	jbd2_journal_init_transaction_limits(journal);
>>
>> @@ -2378,9 +2380,13 @@ void jbd2_journal_clear_features(journal_t *journal, unsigned long compat,
>>
>>   	sb = journal->j_superblock;
>>
>> +	lock_buffer(journal->j_sb_buffer);
>>   	sb->s_feature_compat    &= ~cpu_to_be32(compat);
>>   	sb->s_feature_ro_compat &= ~cpu_to_be32(ro);
>>   	sb->s_feature_incompat  &= ~cpu_to_be32(incompat);
>> +	if (jbd2_journal_has_csum_v2or3(journal))
>> +		sb->s_checksum = jbd2_superblock_csum(sb);
>> +	unlock_buffer(journal->j_sb_buffer);
>>   	jbd2_journal_init_transaction_limits(journal);
>>   }
>>   EXPORT_SYMBOL(jbd2_journal_clear_features);
>> --
>> 2.34.1
>>