Re: [PATCH] f2fs: fix to do sanity check for inline inode
From: Chao Yu
Date: Thu May 05 2022 - 10:34:02 EST
On 2022/5/5 11:31, Jaegeuk Kim wrote:
On 05/05, Chao Yu wrote:
On 2022/5/5 5:28, Jaegeuk Kim wrote:
On 04/28, Chao Yu wrote:
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: stable@xxxxxxxxxxxxxxx
Reported-by: Ming Yan <yanming@xxxxxxxxxx>
Signed-off-by: Chao Yu <chao.yu@xxxxxxxx>
---
fs/f2fs/f2fs.h | 7 +++++++
fs/f2fs/inode.c | 3 +--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 27aa93caec06..64c511b498cc 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4173,6 +4173,13 @@ 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 (file_is_encrypt(inode) || file_is_verity(inode) ||
+ F2FS_I(inode)->i_flags & F2FS_COMPR_FL)
+ return true;
return f2fs_encrypted_file(inode) || fsverity_active(inode) ||
f2fs_compressed_file(inode);
}
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 83639238a1fe..234b8ed02644 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -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)) {
It seems f2fs_may_inline_data() is breaking the atomic write case. Please fix.
sanity_check_inode() change only affect f2fs_iget(), during inode initialization,
file should not be set as atomic one, right?
I didn't see any failure during 'f2fs_io write atomic_write' testcase... could you
please provide me detail of the testcase?
I just applied this into my device and was getting lots of the below error
messages resulting in open failures of database files.
Could you please help to apply below patch and dump the log?
From: Chao Yu <chao@xxxxxxxxxx>
Subject: [PATCH] f2fs: fix to do sanity check for inline inode
Signed-off-by: Chao Yu <chao.yu@xxxxxxxx>
---
fs/f2fs/f2fs.h | 7 +++++++
fs/f2fs/inode.c | 11 +++++++----
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 0f8c426aed50..13a9212d6cb6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4159,6 +4159,13 @@ 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 (file_is_encrypt(inode) || file_is_verity(inode) ||
+ F2FS_I(inode)->i_flags & F2FS_COMPR_FL)
+ return true;
return f2fs_encrypted_file(inode) || fsverity_active(inode) ||
f2fs_compressed_file(inode);
}
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 02630c17da93..a98614a24ad0 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -276,11 +276,14 @@ 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);
+ f2fs_warn(sbi, "%s: inode (ino=%lx, mode=%u) reason(%d, %llu, %ld, %d, %d, %lu) should not have inline_data, run fsck to fix",
+ __func__, inode->i_ino, inode->i_mode,
+ f2fs_is_atomic_file(inode),
+ i_size_read(inode), MAX_INLINE_DATA(inode),
+ file_is_encrypt(inode), file_is_verity(inode),
+ F2FS_I(inode)->i_flags & F2FS_COMPR_FL);
return false;
}
--
2.32.0
Thanks,
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);
--
2.25.1