linux-ext4 - Re: [syzbot] [ext4?] possible deadlock in ext4_destroy_inline

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251201232552.GA89435@frogsfrogsfrogs>
Date: Mon, 1 Dec 2025 15:25:52 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: Andreas Dilger <adilger@...ger.ca>
Cc: Theodore Tso <tytso@....edu>,
	syzbot <syzbot+bb2455d02bda0b5701e3@...kaller.appspotmail.com>,
	linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
	syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [ext4?] possible deadlock in ext4_destroy_inline_data
 (2)

On Mon, Dec 01, 2025 at 04:17:02PM -0700, Andreas Dilger wrote:
> On Dec 1, 2025, at 9:16 AM, Theodore Tso <tytso@....edu> wrote:
> > 
> > That being said, we probably should just not try to expand the inode's
> > extra size while evicting the inode.  In practice we don't actually do
> > this since we haven't expanded the inode's extra size space in over a
> > decade, and so this only happens in a debugging mount option that
> > syzbot helpfully uses, and not in real life.
> 
> I think we would regret removing this if/when we *do* expand the inode
> size.  We used this functionality to upgrade filesystems online when
> i_projid was first added and users suddenly wanted to use project quotas.
> If we need some new inode field in the future it will be good to have it.

Or expand extra_isize only when someone tries to set an inode field that
actually requires it?  e.g. whenever setting the project id?

--D

> > Also, there's no real point in doing this on the evict path,
> > especially if the inode is about to be released as part of the
> > eviction.
> 
> This could check in ext4_orphan_cleanup()->ext4_evict_inode() path
> that this is orphan cleanup with EXT4_ORPHAN_FS and skip the expansion?
> As you write, it doesn't make sense to do that when the file is being
> deleted anyway.  Something like the following, which adds unlikely() to
> that branch since it may happen only once or never in the lifetime of
> any inode:
> 
> Cheers, Andreas
> ---
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index e99306a8f47c..ae48748decc5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -6481,7 +6490,8 @@ int __ext4_mark_inode_dirty(handle_t *handle,
>  	if (err)
>  		goto out;
> -	if (EXT4_I(inode)->i_extra_isize < sbi->s_want_extra_isize)
> +	if (unlikely(EXT4_I(inode)->i_extra_isize < sbi->s_want_extra_isize &&
> +		     !(sbi->s_mount_state & EXT4_ORPHAN_FS)))
>  		ext4_try_to_expand_extra_isize(inode, sbi->s_want_extra_isize,
>  					       iloc, handle);
> 
> 
> 
> 
> 
>