linux-kernel - Re: [PATCH] f2fs: invalidate dentry cache on failed whiteout creation

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: <a6dc6438-7aaa-4c21-8df7-ecd618b826ea@kernel.org>
Date: Mon, 27 Oct 2025 20:12:43 +0800
From: Chao Yu <chao@...nel.org>
To: Deepanshu Kartikey <kartikey406@...il.com>, jaegeuk@...nel.org,
 linux-f2fs-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Cc: chao@...nel.org, syzbot+632cf32276a9a564188d@...kaller.appspotmail.com
Subject: Re: [PATCH] f2fs: invalidate dentry cache on failed whiteout creation

On 10/23/25 07:33, Deepanshu Kartikey wrote:
> F2FS can mount filesystems with corrupted directory depth values that
> get runtime-clamped to MAX_DIR_HASH_DEPTH. When RENAME_WHITEOUT
> operations are performed on such directories, f2fs_rename performs
> directory modifications (updating target entry and deleting source
> entry) before attempting to add the whiteout entry via f2fs_add_link.
> 
> If f2fs_add_link fails due to the corrupted directory structure, the
> function returns an error to VFS, but the partial directory
> modifications have already been committed to disk. VFS assumes the
> entire rename operation failed and does not update the dentry cache,
> leaving stale mappings.

Nice catch!

More detail is in error path, vfs won't call d_exchange to exchange old_dentry
and new_dentry, result in new_dentry still link to new_inode which has zeroed
nlink, once we try to remove this new_inode, it will trigger the bug.

Can you please add above info into your description?

> 
> This causes subsequent operations to use cached dentry information that
> no longer matches the on-disk state. When a second rename targets the
> same entry, VFS attempts to decrement i_nlink on the stale inode, which
> may already have i_nlink=0, triggering a WARNING in drop_nlink().
> 
> Example sequence:
> 1. First rename (RENAME_WHITEOUT): file2 → file1
>    - f2fs updates file1 entry on disk (points to inode 8)
>    - f2fs deletes file2 entry on disk
>    - f2fs_add_link(whiteout) fails (corrupted directory)
>    - Returns error to VFS
>    - VFS cache still has: file1 → inode 7 (stale!)
> 
> 2. Second rename: file3 → file1
>    - VFS uses stale cache: file1 → inode 7
>    - Tries to drop_nlink on inode 7 (i_nlink already 0)
>    - WARNING in drop_nlink()
> 
> Fix this by explicitly invalidating old_dentry and new_dentry when
> f2fs_add_link fails during whiteout creation. This forces VFS to
> refresh from disk on subsequent operations, ensuring cache consistency
> even when the rename partially succeeds.
> 
> Reproducer:
> 1. Mount F2FS image with corrupted i_current_depth
> 2. renameat2(file2, file1, RENAME_WHITEOUT)
> 3. renameat2(file3, file1, 0)
> 4. System triggers WARNING in drop_nlink()
> 

Can you please add Fixes flag and Cc stable@...nel.org?

Otherwise it looks good to me.

Thanks,

> Reported-by: syzbot+632cf32276a9a564188d@...kaller.appspotmail.com
> Tested-by: syzbot+632cf32276a9a564188d@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=632cf32276a9a564188d
> Signed-off-by: Deepanshu Kartikey <kartikey406@...il.com>
> ---
>  fs/f2fs/namei.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index b882771e4699..712479b7b93d 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -1053,9 +1053,11 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  	if (whiteout) {
>  		set_inode_flag(whiteout, FI_INC_LINK);
>  		err = f2fs_add_link(old_dentry, whiteout);
> -		if (err)
> +		if (err) {
> +			d_invalidate(old_dentry);
> +			d_invalidate(new_dentry);
>  			goto put_out_dir;
> -
> +		}
>  		spin_lock(&whiteout->i_lock);
>  		whiteout->i_state &= ~I_LINKABLE;
>  		spin_unlock(&whiteout->i_lock);