linux-kernel - Re: [f2fs-dev] [PATCH] f2fs: speed up f2fs_empty

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] [day] [month] [year] [list]

Message-ID: <b42f9a77-942b-9e55-2637-93821255b370@kernel.org>
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