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@vger.kernel.org Reported-by: Ming Yan yanming@tju.edu.cn Signed-off-by: Chao Yu chao.yu@oppo.com --- 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)) { 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);
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@vger.kernel.org Reported-by: Ming Yan yanming@tju.edu.cn Signed-off-by: Chao Yu chao.yu@oppo.com
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;} 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.
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
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@vger.kernel.org Reported-by: Ming Yan yanming@tju.edu.cn Signed-off-by: Chao Yu chao.yu@oppo.com
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;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?
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
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@vger.kernel.org Reported-by: Ming Yan yanming@tju.edu.cn Signed-off-by: Chao Yu chao.yu@oppo.com
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;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.
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
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@vger.kernel.org Reported-by: Ming Yan yanming@tju.edu.cn Signed-off-by: Chao Yu chao.yu@oppo.com
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;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@kernel.org Subject: [PATCH] f2fs: fix to do sanity check for inline inode
Signed-off-by: Chao Yu chao.yu@oppo.com --- 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; }
Ping,
On 2022/5/5 22:33, Chao Yu wrote:
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@vger.kernel.org Reported-by: Ming Yan yanming@tju.edu.cn Signed-off-by: Chao Yu chao.yu@oppo.com
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@kernel.org Subject: [PATCH] f2fs: fix to do sanity check for inline inode
Signed-off-by: Chao Yu chao.yu@oppo.com
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;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;
On 05/08, Chao Yu wrote:
Ping,
This is in my TODO list, but will take some time. Sorry.
On 2022/5/5 22:33, Chao Yu wrote:
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@vger.kernel.org Reported-by: Ming Yan yanming@tju.edu.cn Signed-off-by: Chao Yu chao.yu@oppo.com
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@kernel.org Subject: [PATCH] f2fs: fix to do sanity check for inline inode
Signed-off-by: Chao Yu chao.yu@oppo.com
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;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;
On 05/09, Jaegeuk Kim wrote:
On 05/08, Chao Yu wrote:
Ping,
This is in my TODO list, but will take some time. Sorry.
Got this.
sanity_check_inode: inode (ino=1125d, mode=41471) reason(0, 98, 3452, 4, 0, 0) should not have inline_data, run fsck to fix
Ok, this is a symlink, which was encrypted having inline_data.
On 2022/5/5 22:33, Chao Yu wrote:
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@vger.kernel.org > Reported-by: Ming Yan yanming@tju.edu.cn > Signed-off-by: Chao Yu chao.yu@oppo.com > --- > 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@kernel.org Subject: [PATCH] f2fs: fix to do sanity check for inline inode
Signed-off-by: Chao Yu chao.yu@oppo.com
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;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;
Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
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@vger.kernel.org Reported-by: Ming Yan yanming@tju.edu.cn Signed-off-by: Chao Yu chao.yu@oppo.com
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;} 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)) {
BTW, why can't we just check the above on-disk fields here only?
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
linux-stable-mirror@lists.linaro.org
-
Chao Yu -
Jaegeuk Kim