f2fs: fix to do sanity check for inline inode

PATCH Superseded Chao Yu 2 participants 3 messages May 14, 2022 – May 15, 2022

Subsystems (2)

Stable branch F2FS file system

lore

Share & tools

lore

Chao Yu

author maintainer

2022-05-14 08:01 UTC #0 lore raw

PATCH v2 1/1 f2fs: fix to do sanity check for inline inode

As Yanming reported in bugzilla:

https://bugzilla.kernel.org/show_bug.cgi?id=215895

I have encountered a bug in F2FS file system in kernel v5.17.

The kernel message is shown below:

kernel BUG at fs/inode.c:611!

Call Trace:
 evict+0x282/0x4e0
 __dentry_kill+0x2b2/0x4d0
 dput+0x2dd/0x720
 do_renameat2+0x596/0x970
 __x64_sys_rename+0x78/0x90
 do_syscall_64+0x3b/0x90

The root cause is: fuzzed inode has both inline_data flag and encrypted flag, so after it was deleted by rename(), during f2fs_evict_inode(), it will cause inline data conversion due to flags confilction, then page cache will be polluted and trigger panic in clear_inode().

This patch tries to fix the issue by do more sanity checks for inline data inode in sanity_check_inode().

Cc [email protected]

Reported-by Ming Yan <[email protected]>

Signed-off-by Chao Yu <[email protected]>

---
v2:
- fix to check inode type in f2fs_post_read_required()

 fs/f2fs/f2fs.h  | 8 ++++++++
 fs/f2fs/inode.c | 3 +--
 2 files changed, 9 insertions(+), 2 deletions(-)

fs/f2fs/f2fs.h +8 -0

index 492af5b96de1..0dc2461ef02c 100644
@@ -4126,6 +4126,14 @@ static inline void f2fs_set_encrypted_inode(struct inode *inode)
  */
 static inline bool f2fs_post_read_required(struct inode *inode)
 {
+	/*
+	 * used by sanity_check_inode(), when disk layout fields has not
+	 * been synchronized to inmem fields.
+	 */
+	if (S_ISREG(inode->i_mode) && (file_is_encrypt(inode) ||
+		F2FS_I(inode)->i_flags & F2FS_COMPR_FL ||
+		file_is_verity(inode)))
+		return true;
 	return f2fs_encrypted_file(inode) || fsverity_active(inode) ||
 		f2fs_compressed_file(inode);
 }

fs/f2fs/inode.c +1 -2

index 2fce8fa0dac8..5e494c98e3c2 100644
@@ -276,8 +276,7 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page)
 		}
 	}
 
-	if (f2fs_has_inline_data(inode) &&
-			(!S_ISREG(inode->i_mode) && !S_ISLNK(inode->i_mode))) {
+	if (f2fs_has_inline_data(inode) && !f2fs_may_inline_data(inode)) {
 		set_sbi_flag(sbi, SBI_NEED_FSCK);
 		f2fs_warn(sbi, "%s: inode (ino=%lx, mode=%u) should not have inline_data, run fsck to fix",
 			  __func__, inode->i_ino, inode->i_mode);

Signature · 1 line

2.32.0

Bagas Sanjaya

2022-05-14 12:14 UTC #1 lore raw

Quoted 22 lines · Chao Yu

As Yanming reported in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=215895
I have encountered a bug in F2FS file system in kernel v5.17.
The kernel message is shown below:
kernel BUG at fs/inode.c:611!
Call Trace:
 evict+0x282/0x4e0
 __dentry_kill+0x2b2/0x4d0
 dput+0x2dd/0x720
 do_renameat2+0x596/0x970
 __x64_sys_rename+0x78/0x90
 do_syscall_64+0x3b/0x90
The root cause is: fuzzed inode has both inline_data flag and encrypted flag, so after it was deleted by rename(), during f2fs_evict_inode(), it will cause inline data conversion due to flags confilction, then page cache will be polluted and trigger panic in clear_inode().

Quoted 6 lines

This patch tries to fix the issue by do more sanity checks for inline data inode in sanity_check_inode().
Cc [email protected]
Reported-by Ming Yan <[email protected]>
Signed-off-by Chao Yu <[email protected]>

Hi Chao,

I think the patch message can be reworked , like below:

Yanming reported a kernel bug in Bugzilla kernel, which can be reproduced. The bug message is:

kernel BUG at fs/inode.c:611!

Call Trace:
 evict+0x282/0x4e0
 __dentry_kill+0x2b2/0x4d0
 dput+0x2dd/0x720
 do_renameat2+0x596/0x970
 __x64_sys_rename+0x78/0x90
 do_syscall_64+0x3b/0x90

The bug is due to fuzzed inode has both inline_data and encrypted flags. During f2fs_evict_inode(), after the inode was deleted by rename(), it will cause inline data conversion due to conflicting flags. The page cache will be polluted and the panic will be triggered in clear_inode().

Try fixing the bug by doing more sanity checks for inline data inode in sanity_check_inode().

Thanks.

Signature · 1 line

An old man doll... just what I always wanted! - Clara

Chao Yu

author maintainer

2022-05-15 08:57 UTC #2 lore raw

Quoted 26 lines · Bagas Sanjaya

Quoted 25 lines · Chao Yu
As Yanming reported in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=215895
I have encountered a bug in F2FS file system in kernel v5.17.
The kernel message is shown below:
kernel BUG at fs/inode.c:611!
Call Trace:
  evict+0x282/0x4e0
  __dentry_kill+0x2b2/0x4d0
  dput+0x2dd/0x720
  do_renameat2+0x596/0x970
  __x64_sys_rename+0x78/0x90
  do_syscall_64+0x3b/0x90
The root cause is: fuzzed inode has both inline_data flag and encrypted flag, so after it was deleted by rename(), during f2fs_evict_inode(), it will cause inline data conversion due to flags confilction, then page cache will be polluted and trigger panic in clear_inode().
This patch tries to fix the issue by do more sanity checks for inline data inode in sanity_check_inode().

Quoted 7 lines

Quoted 3 lines
Cc [email protected]
Reported-by Ming Yan <[email protected]>
Signed-off-by Chao Yu <[email protected]>
Hi Chao,
I think the patch message can be reworked , like below:

Hi Bagas,

Thanks a lot for your cleanup. :)

Quoted 3 lines

Yanming reported a kernel bug in Bugzilla kernel, which can be reproduced. The bug message is:

I will keep the link for backtrace.

Quoted 10 lines

kernel BUG at fs/inode.c:611!

Call Trace:
  evict+0x282/0x4e0
  __dentry_kill+0x2b2/0x4d0
  dput+0x2dd/0x720
  do_renameat2+0x596/0x970
  __x64_sys_rename+0x78/0x90
  do_syscall_64+0x3b/0x90

Quoted 2 lines

The bug is due to fuzzed inode has both inline_data and encrypted flags. During f2fs_evict_inode(), after the inode was deleted by rename(), it

I prefer "during f2fs_evict_inode(), as inode was deleted by rename()"

Quoted 5 lines

will cause inline data conversion due to conflicting flags. The page cache will be polluted and the panic will be triggered in clear_inode().
Try fixing the bug by doing more sanity checks for inline data inode in sanity_check_inode().

Let me revise in v3.

Thanks,

Quoted 3 lines

Thanks.