* Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb
2025-10-28 6:47 [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb Ye Bin
@ 2025-10-29 8:45 ` Zhang Yi
2025-10-29 11:44 ` yebin
2025-10-29 14:55 ` Darrick J. Wong
2025-10-30 10:46 ` Jan Kara
2 siblings, 1 reply; 7+ messages in thread
From: Zhang Yi @ 2025-10-29 8:45 UTC (permalink / raw)
To: Ye Bin, linux-ext4; +Cc: jack, tytso, adilger.kernel
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);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb
2025-10-29 8:45 ` Zhang Yi
@ 2025-10-29 11:44 ` yebin
0 siblings, 0 replies; 7+ messages in thread
From: yebin @ 2025-10-29 11:44 UTC (permalink / raw)
To: Zhang Yi, linux-ext4; +Cc: jack, tytso, adilger.kernel
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);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb
2025-10-28 6:47 [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb Ye Bin
2025-10-29 8:45 ` Zhang Yi
@ 2025-10-29 14:55 ` Darrick J. Wong
2025-10-30 2:13 ` Zhang Yi
2025-10-30 10:46 ` Jan Kara
2 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2025-10-29 14:55 UTC (permalink / raw)
To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, jack
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
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb
2025-10-29 14:55 ` Darrick J. Wong
@ 2025-10-30 2:13 ` Zhang Yi
0 siblings, 0 replies; 7+ messages in thread
From: Zhang Yi @ 2025-10-30 2:13 UTC (permalink / raw)
To: Darrick J. Wong, Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, jack
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
>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb
2025-10-28 6:47 [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb Ye Bin
2025-10-29 8:45 ` Zhang Yi
2025-10-29 14:55 ` Darrick J. Wong
@ 2025-10-30 10:46 ` Jan Kara
2025-10-31 1:47 ` yebin
2 siblings, 1 reply; 7+ messages in thread
From: Jan Kara @ 2025-10-30 10:46 UTC (permalink / raw)
To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, jack
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] jbd2: fix the inconsistency between checksum and data in memory for journal sb
2025-10-30 10:46 ` Jan Kara
@ 2025-10-31 1:47 ` yebin
0 siblings, 0 replies; 7+ messages in thread
From: yebin @ 2025-10-31 1:47 UTC (permalink / raw)
To: Jan Kara; +Cc: tytso, adilger.kernel, linux-ext4
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
>>
^ permalink raw reply [flat|nested] 7+ messages in thread