2023-11-28 09:25:38

by Chao Yu

Subject: [PATCH] f2fs: fix to avoid dirent corruption

As Al reported in link[1]:

f2fs_rename()
...
if (old_dir != new_dir && !whiteout)
f2fs_set_link(old_inode, old_dir_entry,
old_dir_page, new_dir);
else
f2fs_put_page(old_dir_page, 0);

You want correct inumber in the ".." link. And cross-directory
rename does move the source to new parent, even if you'd been asked
to leave a whiteout in the old place.

[1] https://lore.kernel.org/all/20231017055040.GN800259@ZenIV/

With below testcase, it may cause dirent corruption, due to it missed
to call f2fs_set_link() to update ".." link to new directory.
- mkdir -p dir/foo
- renameat2 -w dir/foo bar

[ASSERT] (__chk_dots_dentries:1421) --> Bad inode number[0x4] for '..', parent parent ino is [0x3]
[FSCK] other corrupted bugs [Fail]

Fixes: 7e01e7ad746b ("f2fs: support RENAME_WHITEOUT")
Cc: Jan Kara <[email protected]>
Reported-by: Al Viro <[email protected]>
Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/namei.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 3b1793cfb002..ede6afb81762 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -1105,7 +1105,7 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
}

if (old_dir_entry) {
- if (old_dir != new_dir && !whiteout)
+ if (old_dir != new_dir)
f2fs_set_link(old_inode, old_dir_entry,
old_dir_page, new_dir);
else
--
2.40.1

2023-11-28 11:03:20

by Jan Kara

Subject: Re: [PATCH] f2fs: fix to avoid dirent corruption

On Tue 28-11-23 17:25:16, Chao Yu wrote:
> As Al reported in link[1]:
>
> f2fs_rename()
> ...
> if (old_dir != new_dir && !whiteout)
> f2fs_set_link(old_inode, old_dir_entry,
> old_dir_page, new_dir);
> else
> f2fs_put_page(old_dir_page, 0);
>
> You want correct inumber in the ".." link. And cross-directory
> rename does move the source to new parent, even if you'd been asked
> to leave a whiteout in the old place.
>
> [1] https://lore.kernel.org/all/20231017055040.GN800259@ZenIV/
>
> With below testcase, it may cause dirent corruption, due to it missed
> to call f2fs_set_link() to update ".." link to new directory.
> - mkdir -p dir/foo
> - renameat2 -w dir/foo bar
>
> [ASSERT] (__chk_dots_dentries:1421) --> Bad inode number[0x4] for '..', parent parent ino is [0x3]
> [FSCK] other corrupted bugs [Fail]
>
> Fixes: 7e01e7ad746b ("f2fs: support RENAME_WHITEOUT")
> Cc: Jan Kara <[email protected]>
> Reported-by: Al Viro <[email protected]>
> Signed-off-by: Chao Yu <[email protected]>

Thanks for fixing this! Makes sense to me so feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/f2fs/namei.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 3b1793cfb002..ede6afb81762 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -1105,7 +1105,7 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
> }
>
> if (old_dir_entry) {
> - if (old_dir != new_dir && !whiteout)
> + if (old_dir != new_dir)
> f2fs_set_link(old_inode, old_dir_entry,
> old_dir_page, new_dir);
> else
> --
> 2.40.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-11-30 18:31:38

by patchwork-bot+f2fs

Subject: Re: [f2fs-dev] [PATCH] f2fs: fix to avoid dirent corruption

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <[email protected]>:

On Tue, 28 Nov 2023 17:25:16 +0800 you wrote:
> As Al reported in link[1]:
>
> f2fs_rename()
> ...
> if (old_dir != new_dir && !whiteout)
> f2fs_set_link(old_inode, old_dir_entry,
> old_dir_page, new_dir);
> else
> f2fs_put_page(old_dir_page, 0);
>
> [...]

Here is the summary with links:
- [f2fs-dev] f2fs: fix to avoid dirent corruption
https://git.kernel.org/jaegeuk/f2fs/c/53edb549565f

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html