[PATCH 6.1] ext4: fix access to uninitialised lock in fc replay path

[PATCH 6.1] ext4: fix access to uninitialised lock in fc replay path

[vgiraud.opensource]

From: "Luis Henriques (SUSE)" <luis.henriques@linux.dev>

commit 23dfdb56581ad92a9967bcd720c8c23356af74c1 upstream.

The following kernel trace can be triggered with fstest generic/629 when
executed against a filesystem with fast-commit feature enabled:

INFO: trying to register non-static key.
The code is fine but needs lockdep annotation, or maybe
you didn't initialize this object before use?
turning off the locking correctness validator.
CPU: 0 PID: 866 Comm: mount Not tainted 6.10.0+ #11
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-3-gd478f380-prebuilt.qemu.org 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0x66/0x90
 register_lock_class+0x759/0x7d0
 __lock_acquire+0x85/0x2630
 ? __find_get_block+0xb4/0x380
 lock_acquire+0xd1/0x2d0
 ? __ext4_journal_get_write_access+0xd5/0x160
 _raw_spin_lock+0x33/0x40
 ? __ext4_journal_get_write_access+0xd5/0x160
 __ext4_journal_get_write_access+0xd5/0x160
 ext4_reserve_inode_write+0x61/0xb0
 __ext4_mark_inode_dirty+0x79/0x270
 ? ext4_ext_replay_set_iblocks+0x2f8/0x450
 ext4_ext_replay_set_iblocks+0x330/0x450
 ext4_fc_replay+0x14c8/0x1540
 ? jread+0x88/0x2e0
 ? rcu_is_watching+0x11/0x40
 do_one_pass+0x447/0xd00
 jbd2_journal_recover+0x139/0x1b0
 jbd2_journal_load+0x96/0x390
 ext4_load_and_init_journal+0x253/0xd40
 ext4_fill_super+0x2cc6/0x3180
...

In the replay path there's an attempt to lock sbi->s_bdev_wb_lock in
function ext4_check_bdev_write_error().  Unfortunately, at this point this
spinlock has not been initialized yet.  Moving it's initialization to an
earlier point in __ext4_fill_super() fixes this splat.

Signed-off-by: Luis Henriques (SUSE) <luis.henriques@linux.dev>
Link: https://patch.msgid.link/20240718094356.7863-1-luis.henriques@linux.dev
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@kernel.org
Signed-off-by: Bruno VERNAY <bruno.vernay@se.com>
Signed-off-by: Victor Giraud <vgiraud.opensource@witekio.com>
---
 fs/ext4/super.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 0b2591c07166..53f1deb049ec 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5264,6 +5264,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 	INIT_LIST_HEAD(&sbi->s_orphan); /* unlinked but open files */
 	mutex_init(&sbi->s_orphan_lock);
 
+	spin_lock_init(&sbi->s_bdev_wb_lock);
+
 	ext4_fast_commit_init(sb);
 
 	sb->s_root = NULL;
@@ -5514,7 +5516,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 	 * Save the original bdev mapping's wb_err value which could be
 	 * used to detect the metadata async write error.
 	 */
-	spin_lock_init(&sbi->s_bdev_wb_lock);
 	errseq_check_and_advance(&sb->s_bdev->bd_inode->i_mapping->wb_err,
 				 &sbi->s_bdev_wb_err);
 	sb->s_bdev->bd_super = sb;
-- 
2.34.1

Re: [PATCH 6.1] ext4: fix access to uninitialised lock in fc replay path

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 23dfdb56581ad92a9967bcd720c8c23356af74c1

WARNING: Author mismatch between patch and upstream commit:
Backport author: vgiraud.opensource@witekio.com
Commit author: Luis Henriques (SUSE)<luis.henriques@linux.dev>


Status in newer kernel trees:
6.13.y | Branch not found
6.12.y | Present (exact SHA1)
6.6.y | Not found
6.1.y | Not found

Note: The patch differs from the upstream commit:
---
1:  23dfdb56581ad ! 1:  366f73b541bff ext4: fix access to uninitialised lock in fc replay path
    @@ Metadata
      ## Commit message ##
         ext4: fix access to uninitialised lock in fc replay path
     
    +    commit 23dfdb56581ad92a9967bcd720c8c23356af74c1 upstream.
    +
         The following kernel trace can be triggered with fstest generic/629 when
         executed against a filesystem with fast-commit feature enabled:
     
    @@ Commit message
         Link: https://patch.msgid.link/20240718094356.7863-1-luis.henriques@linux.dev
         Signed-off-by: Theodore Ts'o <tytso@mit.edu>
         Cc: stable@kernel.org
    +    Signed-off-by: Bruno VERNAY <bruno.vernay@se.com>
    +    Signed-off-by: Victor Giraud <vgiraud.opensource@witekio.com>
     
      ## fs/ext4/super.c ##
     @@ fs/ext4/super.c: static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
    @@ fs/ext4/super.c: static int __ext4_fill_super(struct fs_context *fc, struct supe
      	 * used to detect the metadata async write error.
      	 */
     -	spin_lock_init(&sbi->s_bdev_wb_lock);
    - 	errseq_check_and_advance(&sb->s_bdev->bd_mapping->wb_err,
    + 	errseq_check_and_advance(&sb->s_bdev->bd_inode->i_mapping->wb_err,
      				 &sbi->s_bdev_wb_err);
    - 	EXT4_SB(sb)->s_mount_state |= EXT4_ORPHAN_FS;
    + 	sb->s_bdev->bd_super = sb;
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.1.y        |  Success    |  Success   |

Re: [PATCH 6.1] ext4: fix access to uninitialised lock in fc replay path

[Greg KH]

On Wed, Jan 29, 2025 at 11:10:46AM +0100, vgiraud.opensource@witekio.com wrote:
> From: "Luis Henriques (SUSE)" <luis.henriques@linux.dev>
> 
> commit 23dfdb56581ad92a9967bcd720c8c23356af74c1 upstream.
> 
> The following kernel trace can be triggered with fstest generic/629 when
> executed against a filesystem with fast-commit feature enabled:
> 
> INFO: trying to register non-static key.
> The code is fine but needs lockdep annotation, or maybe
> you didn't initialize this object before use?
> turning off the locking correctness validator.
> CPU: 0 PID: 866 Comm: mount Not tainted 6.10.0+ #11
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-3-gd478f380-prebuilt.qemu.org 04/01/2014
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x66/0x90
>  register_lock_class+0x759/0x7d0
>  __lock_acquire+0x85/0x2630
>  ? __find_get_block+0xb4/0x380
>  lock_acquire+0xd1/0x2d0
>  ? __ext4_journal_get_write_access+0xd5/0x160
>  _raw_spin_lock+0x33/0x40
>  ? __ext4_journal_get_write_access+0xd5/0x160
>  __ext4_journal_get_write_access+0xd5/0x160
>  ext4_reserve_inode_write+0x61/0xb0
>  __ext4_mark_inode_dirty+0x79/0x270
>  ? ext4_ext_replay_set_iblocks+0x2f8/0x450
>  ext4_ext_replay_set_iblocks+0x330/0x450
>  ext4_fc_replay+0x14c8/0x1540
>  ? jread+0x88/0x2e0
>  ? rcu_is_watching+0x11/0x40
>  do_one_pass+0x447/0xd00
>  jbd2_journal_recover+0x139/0x1b0
>  jbd2_journal_load+0x96/0x390
>  ext4_load_and_init_journal+0x253/0xd40
>  ext4_fill_super+0x2cc6/0x3180
> ...
> 
> In the replay path there's an attempt to lock sbi->s_bdev_wb_lock in
> function ext4_check_bdev_write_error().  Unfortunately, at this point this
> spinlock has not been initialized yet.  Moving it's initialization to an
> earlier point in __ext4_fill_super() fixes this splat.
> 
> Signed-off-by: Luis Henriques (SUSE) <luis.henriques@linux.dev>
> Link: https://patch.msgid.link/20240718094356.7863-1-luis.henriques@linux.dev
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Cc: stable@kernel.org
> Signed-off-by: Bruno VERNAY <bruno.vernay@se.com>
> Signed-off-by: Victor Giraud <vgiraud.opensource@witekio.com>
> ---
>  fs/ext4/super.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

You forgot about 6.6.y, please ALWAYS check if a commit is in a newer
kernel or not, because we can't take them if the are not there also.

thanks,

greg k-h