Re: [f2fs-dev] f2fs_empty_dir() can be extremely slow on malicious disk images

From: Chao Yu
Date: Wed Nov 02 2022 - 19:19:20 EST


On 2022/11/2 23:12, Chao Yu wrote:
On 2022/11/2 14:17, Eric Biggers wrote:
[+f2fs list and maintainers]

Thanks for the forwarding.

[changed subject from "INFO: task hung in fscrypt_ioctl_set_policy"]

On Mon, Oct 31, 2022 at 10:18:02PM +0800, Wei Chen wrote:
Dear Linux developers,

Here is the link to the reproducers.

C reproducer: https://drive.google.com/file/d/1mduYsYuoOKemH3qkvpDQwnAHAaaLUp0Y/view?usp=share_link
Syz reproducer:
https://drive.google.com/file/d/1mu-_w7dy_562vWRlQvTRbcBjG4_G7b2L/view?usp=share_link

The bug persists in the latest commit, v5.15.76 (4f5365f77018). I hope
it is helpful to you.

[ 1782.137186][ T30] INFO: task a.out:6910 blocked for more than 143 seconds.
[ 1782.139217][ T30] Not tainted 5.15.76 #5
[ 1782.140388][ T30] "echo 0 >
/proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 1782.142524][ T30] task:a.out state:D stack:14296 pid:
6910 ppid: 6532 flags:0x00004004
[ 1782.144799][ T30] Call Trace:
[ 1782.145623][ T30] <TASK>
[ 1782.146316][ T30] __schedule+0x3e8/0x1850
[ 1782.152029][ T30] ? mark_held_locks+0x49/0x70
[ 1782.153533][ T30] ? mark_held_locks+0x10/0x70
[ 1782.154759][ T30] ? __down_write_common.part.14+0x31f/0x7b0
[ 1782.156159][ T30] schedule+0x4e/0xe0
[ 1782.158314][ T30] __down_write_common.part.14+0x324/0x7b0
[ 1782.159704][ T30] ? fscrypt_ioctl_set_policy+0xe0/0x200
[ 1782.161050][ T30] fscrypt_ioctl_set_policy+0xe0/0x200
[ 1782.162330][ T30] __f2fs_ioctl+0x9d6/0x45e0
[ 1782.163417][ T30] f2fs_ioctl+0x64/0x240
[ 1782.164404][ T30] ? __f2fs_ioctl+0x45e0/0x45e0
[ 1782.165554][ T30] __x64_sys_ioctl+0xb6/0x100
[ 1782.166662][ T30] do_syscall_64+0x34/0xb0
[ 1782.169947][ T30] entry_SYSCALL_64_after_hwframe+0x61/0xcb

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.

I'll look into this more if needed, but Jaegeuk and Chao, do you happen to have
any ideas for how f2fs_empty_dir() should be fixed? Is there an easy way to
just iterate through the blocks that are actually allocated?


Sorry, I mean:

From 07f662ca6bd2a0991961ea42932ce90f19e74624 Mon Sep 17 00:00:00 2001
From: Chao Yu <chao@xxxxxxxxxx>
Date: Wed, 2 Nov 2022 12:02:08 +0800
Subject: [RFC v2] f2fs: speed up f2fs_empty_dir()

Signed-off-by: Chao Yu <chao@xxxxxxxxxx>
---
fs/f2fs/dir.c | 48 ++++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 21960a899b6a..569f7304e3e6 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -956,38 +956,46 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,

bool f2fs_empty_dir(struct inode *dir)
{
- unsigned long bidx;
struct page *dentry_page;
unsigned int bit_pos;
struct f2fs_dentry_block *dentry_blk;
- unsigned long nblock = dir_blocks(dir);
+ pgoff_t index;

if (f2fs_has_inline_dentry(dir))
return f2fs_empty_inline_dir(dir);

- for (bidx = 0; bidx < nblock; bidx++) {
- dentry_page = f2fs_get_lock_data_page(dir, bidx, false);
- if (IS_ERR(dentry_page)) {
- if (PTR_ERR(dentry_page) == -ENOENT)
- continue;
- else
- return false;
- }
+ dentry_page = f2fs_get_lock_data_page(dir, 0, false);
+ if (IS_ERR(dentry_page)) {
+ if (PTR_ERR(dentry_page) == -ENOENT)
+ return true;
+ return false;
+ }

- dentry_blk = page_address(dentry_page);
- if (bidx == 0)
- bit_pos = 2;
- else
- bit_pos = 0;
- bit_pos = find_next_bit_le(&dentry_blk->dentry_bitmap,
- NR_DENTRY_IN_BLOCK,
- bit_pos);
+ dentry_blk = page_address(dentry_page);
+ bit_pos = find_next_bit_le(&dentry_blk->dentry_bitmap,
+ NR_DENTRY_IN_BLOCK, 2);
+ f2fs_put_page(dentry_page, 1);
+
+ if (bit_pos < NR_DENTRY_IN_BLOCK)
+ return false;

- f2fs_put_page(dentry_page, 1);
+ for (index = 1; index < dir_blocks(dir);) {
+ struct dnode_of_data dn;
+ int err;

- if (bit_pos < NR_DENTRY_IN_BLOCK)
+ set_new_dnode(&dn, dir, NULL, NULL, 0);
+ err = f2fs_get_dnode_of_data(&dn, index, LOOKUP_NODE);
+ if (err && err != -ENOENT) {
return false;
+ } else if (err == -ENOENT) {
+ index = f2fs_get_next_page_offset(&dn, index);
+ continue;
+ }
+ f2fs_put_dnode(&dn);
+
+ return false;
}
+
return true;
}

--
2.36.1