lkml.org 
[lkml]   [2022]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [f2fs-dev] [PATCH] f2fs: speed up f2fs_empty_dir()
On Sun, Nov 06, 2022 at 05:48:55PM +0800, Chao Yu wrote:
> Wei Chen reports a kernel bug as blew:
>
> INFO: task syz-executor.0:29056 blocked for more than 143 seconds.
> Not tainted 5.15.0-rc5 #1
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> task:syz-executor.0 state:D stack:14632 pid:29056 ppid: 6574 flags:0x00000004
> Call Trace:
> __schedule+0x4a1/0x1720
> schedule+0x36/0xe0
> rwsem_down_write_slowpath+0x322/0x7a0
> fscrypt_ioctl_set_policy+0x11f/0x2a0
> __f2fs_ioctl+0x1a9f/0x5780
> f2fs_ioctl+0x89/0x3a0
> __x64_sys_ioctl+0xe8/0x140
> do_syscall_64+0x34/0xb0
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Eric did some investigation on this issue, quoted from reply of Eric:
>
> "Well, the quality of this bug report has a lot to be desired (not on
> upstream kernel, reproducer is full of totally irrelevant stuff, not
> sent to the mailing list of the filesystem whose disk image is being
> fuzzed, etc.). But what is going on is that f2fs_empty_dir() doesn't
> consider the case of a directory with an extremely large i_size on a
> malicious disk image.
>
> Specifically, the reproducer mounts an f2fs image with a directory
> that has an i_size of 14814520042850357248, then calls
> FS_IOC_SET_ENCRYPTION_POLICY on it.
>
> That results in a call to f2fs_empty_dir() to check whether the
> directory is empty. f2fs_empty_dir() then iterates through all
> 3616826182336513 blocks the directory allegedly contains to check
> whether any contain anything. i_rwsem is held during this, so
> anything else that tries to take it will hang."
>
> In order to solve this issue, let's use f2fs_get_next_page_offset()
> to speed up iteration by skipping holes for all below functions:
> - f2fs_empty_dir
> - f2fs_readdir
> - find_in_level
>
> The way why we can speed up iteration was described in
> 'commit 3cf4574705b4 ("f2fs: introduce get_next_page_offset to speed
> up SEEK_DATA")'.
>
> Meanwhile, in f2fs_empty_dir(), let's use f2fs_find_data_page()
> instead f2fs_get_lock_data_page(), due to i_rwsem was held in
> caller of f2fs_empty_dir(), there shouldn't be any races, so it's
> fine to not lock dentry page during lookuping dirents in the page.
>
> Link: https://lore.kernel.org/lkml/536944df-a0ae-1dd8-148f-510b476e1347@kernel.org/T/
> Reported-by: Wei Chen <harperchen1110@gmail.com>
> Cc: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> fs/f2fs/data.c | 17 ++++++++++++-----
> fs/f2fs/dir.c | 34 ++++++++++++++++++++++++----------
> fs/f2fs/f2fs.h | 5 +++--
> fs/f2fs/gc.c | 4 ++--
> 4 files changed, 41 insertions(+), 19 deletions(-)

Thanks. I'm not an expert on all the details, but this patch looks good to me.

Given that it optimizes lookups and readdirs too, a better title for the patch
might be something like "f2fs: optimize iteration over sparse directories".

- Eric

\
 
 \ /
  Last update: 2022-11-07 19:30    [W:0.148 / U:1.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site