[PATCH] ext4: skip extra isize expansion while unmounting

[PATCH] ext4: skip extra isize expansion while unmounting

[Hemanth Selam]

syzbot reports a WARN from ext4_xattr_inode_create() reached through the
unmount path:

  EXT4-fs warning (device loop0): ext4_xattr_inode_create:1485: refuse to
  create EA inode when umounting
  WARNING: fs/ext4/xattr.c:1486 at ext4_xattr_inode_lookup_create
   ext4_xattr_block_set
   ext4_expand_extra_isize_ea
   __ext4_expand_extra_isize
   __ext4_mark_inode_dirty
   ext4_dirty_inode
   __mark_inode_dirty
   sync_lazytime
   iput
   dentry_kill
   shrink_dentry_list
   shrink_dcache_for_umount
   generic_shutdown_super
   kill_block_super
   ext4_kill_sb

shrink_dcache_for_umount() clears s_root before generic_shutdown_super()
clears SB_ACTIVE, so during the dcache shrink the last iput() of a
lazytime inode still redirties it and reaches the isize expansion.  The
expansion can move xattrs out to a block, and creating the EA inode for
them needs s_root, which ext4_xattr_inode_create() refuses without.

ext4_try_to_expand_extra_isize() already declines to expand when the
superblock is not active, but that test does not cover this window.
Decline while s_root is gone as well.  The expansion is best effort and
__ext4_mark_inode_dirty() ignores its return value, so nothing else
changes; the inode can be expanded on a later mount.

Running the syzbot reproducer for 60 seconds produced 3583 splats before
this change and none after it, with the same number of mount cycles.

Reported-by: syzbot+4b03894b6ec5753ddf24@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4b03894b6ec5753ddf24
Fixes: f31173c19901 ("ext4: refuse to create ea block when umounted")
Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>
---
 fs/ext4/inode.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bd4b778df9eb..6e523a5c8230 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -6598,8 +6598,14 @@ static int ext4_try_to_expand_extra_isize(struct inode *inode,
 	 * When !SB_ACTIVE, iput triggers write_inode_now() which acquires
 	 * s_writepages_rwsem, causing a deadlock with the caller's active
 	 * jbd2 handle (lock order: s_writepages_rwsem -> jbd2_handle).
+	 *
+	 * Skip it while unmounting as well.  shrink_dcache_for_umount()
+	 * clears s_root before generic_shutdown_super() clears SB_ACTIVE, and
+	 * the last iput() of a lazytime inode in that window redirties it and
+	 * lands here.  Moving xattrs out to a block then needs a new EA inode,
+	 * which ext4_xattr_inode_create() refuses without s_root.
 	 */
-	if (unlikely(!(inode->i_sb->s_flags & SB_ACTIVE)))
+	if (unlikely(!(inode->i_sb->s_flags & SB_ACTIVE) || !inode->i_sb->s_root))
 		return -EBUSY;
 
 	/*
-- 
2.43.7

Re: [PATCH] ext4: skip extra isize expansion while unmounting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for a TOCTOU race condition that can lead to a NULL pointer dereference during unmount.
--

--- Patch [1]: [PATCH] ext4: skip extra isize expansion while unmounting ---
commit b6a44628f998028bf5040431bbd5fcff301de3c8
Author: Hemanth Selam <hemanth.selam@gmail.com>

ext4: skip extra isize expansion while unmounting
    
This patch attempts to fix a warning in ext4_xattr_inode_create reached
during unmount. It declines to expand the extra isize when s_root is
cleared during the dcache shrink but before SB_ACTIVE is cleared.

Closes: https://syzkaller.appspot.com/bug?extid=4b03894b6ec5753ddf24

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index bd4b778df9eb2..6e523a5c8230e 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -6598,8 +6598,14 @@ static int ext4_try_to_expand_extra_isize(struct inode *inode,
[ ... ]
> +	 * Skip it while unmounting as well.  shrink_dcache_for_umount()
> +	 * clears s_root before generic_shutdown_super() clears SB_ACTIVE, and
> +	 * the last iput() of a lazytime inode in that window redirties it and
> +	 * lands here.  Moving xattrs out to a block then needs a new EA inode,
> +	 * which ext4_xattr_inode_create() refuses without s_root.
>  	 */
> -	if (unlikely(!(inode->i_sb->s_flags & SB_ACTIVE)))
> +	if (unlikely(!(inode->i_sb->s_flags & SB_ACTIVE) || !inode->i_sb->s_root))
>  		return -EBUSY;
>  
>  	/*

[Severity: High]
This isn't a bug introduced by this patch, but does checking s_root here
without synchronization leave a race open during unmount?

Background workqueues like rsv_conversion_wq and s_ea_inode_work can
process inodes concurrently with unmount. They are not flushed until
sync_filesystem() and ext4_put_super(), which run after
shrink_dcache_for_umount() clears s_root.

If a background thread sees a non-NULL s_root here, could the unmount
thread concurrently clear s_root right after? The background thread would
then proceed into ext4_xattr_inode_create():

fs/ext4/xattr.c:ext4_xattr_inode_create() {
    ...
    ea_inode = ext4_new_inode(handle, inode->i_sb->s_root->d_inode,
                              S_IFREG | 0600, NULL, inode->i_ino + 1, owner,
                              EXT4_EA_INODE_FL);
    ...
}

If s_root is cleared by the unmount thread just before this dereference,
could this result in a NULL pointer dereference on s_root->d_inode? Or if
s_root is cleared right before the warning check inside
ext4_xattr_inode_create(), wouldn't we still hit the original warning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824094512.2436628-1-hemanth.selam@gmail.com?part=1

Re: [PATCH] ext4: skip extra isize expansion while unmounting

[Hemanth Selam]

On Mon, Aug 24, 2026 at 10:01:41AM +0000, sashiko-bot@kernel.org wrote:
> This isn't a bug introduced by this patch, but does checking s_root here
> without synchronization leave a race open during unmount?

The s_root test is advisory, like the SB_ACTIVE test next to it, and the
patch does not change that.  The race you describe is the
check-then-dereference inside ext4_xattr_inode_create() itself, between
the s_root == NULL test and the inode->i_sb->s_root->d_inode dereference
eleven lines below it.  That predates this patch and is untouched by it.

What the patch changes is how often that code is reached.  Without it,
anything that dirties an inode after shrink_dcache_for_umount() has
cleared s_root and before SB_ACTIVE is cleared walks into
ext4_xattr_block_set() and hits the warning.  That is the path syzbot
reproduces, and there the caller is the unmounting task itself - iput() of
a lazytime inode - so no second thread is involved.  With the patch those
callers get -EBUSY first, so the window your scenario needs gets smaller
rather than larger.

Of the two workers: ext4_ea_inode_work() only iput()s EA inodes, which
carry no in-body xattrs, so it never reaches
ext4_xattr_inode_lookup_create().  ext4_end_io_rsv_work() does reach
ext4_mark_inode_dirty() via ext4_convert_unwritten_extents() and is only
flushed in ext4_put_super(), so it can run in that window - and with this
patch it gets -EBUSY there too.

> Or if s_root is cleared right before the warning check inside
> ext4_xattr_inode_create(), wouldn't we still hit the original warning?

Yes, and it returns -EINVAL exactly as it does today.  The patch removes
the deterministic path syzbot found, not that check.  Flushing
rsv_conversion_wq and s_ea_inode_work before shrink_dcache_for_umount()
would close the rest; I can send that separately if it is wanted.

Thanks,
Hemanth

Re: [PATCH] ext4: skip extra isize expansion while unmounting

[Jan Kara]

On Mon 24-08-26 15:15:12, Hemanth Selam wrote:
> syzbot reports a WARN from ext4_xattr_inode_create() reached through the
> unmount path:
> 
>   EXT4-fs warning (device loop0): ext4_xattr_inode_create:1485: refuse to
>   create EA inode when umounting
>   WARNING: fs/ext4/xattr.c:1486 at ext4_xattr_inode_lookup_create
>    ext4_xattr_block_set
>    ext4_expand_extra_isize_ea
>    __ext4_expand_extra_isize
>    __ext4_mark_inode_dirty
>    ext4_dirty_inode
>    __mark_inode_dirty
>    sync_lazytime
>    iput
>    dentry_kill
>    shrink_dentry_list
>    shrink_dcache_for_umount
>    generic_shutdown_super
>    kill_block_super
>    ext4_kill_sb
> 
> shrink_dcache_for_umount() clears s_root before generic_shutdown_super()
> clears SB_ACTIVE, so during the dcache shrink the last iput() of a
> lazytime inode still redirties it and reaches the isize expansion.  The
> expansion can move xattrs out to a block, and creating the EA inode for
> them needs s_root, which ext4_xattr_inode_create() refuses without.
> 
> ext4_try_to_expand_extra_isize() already declines to expand when the
> superblock is not active, but that test does not cover this window.
> Decline while s_root is gone as well.  The expansion is best effort and
> __ext4_mark_inode_dirty() ignores its return value, so nothing else
> changes; the inode can be expanded on a later mount.
> 
> Running the syzbot reproducer for 60 seconds produced 3583 splats before
> this change and none after it, with the same number of mount cycles.
> 
> Reported-by: syzbot+4b03894b6ec5753ddf24@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=4b03894b6ec5753ddf24
> Fixes: f31173c19901 ("ext4: refuse to create ea block when umounted")
> Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/ext4/inode.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index bd4b778df9eb..6e523a5c8230 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -6598,8 +6598,14 @@ static int ext4_try_to_expand_extra_isize(struct inode *inode,
>  	 * When !SB_ACTIVE, iput triggers write_inode_now() which acquires
>  	 * s_writepages_rwsem, causing a deadlock with the caller's active
>  	 * jbd2 handle (lock order: s_writepages_rwsem -> jbd2_handle).
> +	 *
> +	 * Skip it while unmounting as well.  shrink_dcache_for_umount()
> +	 * clears s_root before generic_shutdown_super() clears SB_ACTIVE, and
> +	 * the last iput() of a lazytime inode in that window redirties it and
> +	 * lands here.  Moving xattrs out to a block then needs a new EA inode,
> +	 * which ext4_xattr_inode_create() refuses without s_root.
>  	 */
> -	if (unlikely(!(inode->i_sb->s_flags & SB_ACTIVE)))
> +	if (unlikely(!(inode->i_sb->s_flags & SB_ACTIVE) || !inode->i_sb->s_root))
>  		return -EBUSY;
>  
>  	/*
> -- 
> 2.43.7
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR