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
| ||
|
Date: Tue, 8 Nov 2022 09:06:25 +0800 From: Chao Yu <chao@...nel.org> To: Eric Biggers <ebiggers@...nel.org> Cc: jaegeuk@...nel.org, Wei Chen <harperchen1110@...il.com>, linux-kernel@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net Subject: Re: [f2fs-dev] [PATCH] f2fs: speed up f2fs_empty_dir() On 2022/11/8 2:29, Eric Biggers wrote: > 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@...il.com> >> Cc: Eric Biggers <ebiggers@...gle.com> >> Signed-off-by: Chao Yu <chao@...nel.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". Yes, thanks for your suggestion, will update in v2. Thanks, > > - Eric
Powered by blists - more mailing lists