On 12/18/2025 9:30 AM, Zhiguo Niu wrote:
Chao Yu <[email protected]> 于2025年12月17日周三 16:03写道:

On 12/17/25 09:46, Zhiguo Niu wrote:
Chao Yu <[email protected]> 于2025年12月16日周二 16:49写道:

On 12/16/25 09:36, Zhiguo Niu wrote:
Chao Yu via Linux-f2fs-devel <[email protected]>
于2025年12月15日周一 20:34写道:

In order to avoid loading corrupted nat entry from disk.

Cc: [email protected]
Signed-off-by: Chao Yu <[email protected]>
---
  fs/f2fs/node.c | 9 +++++----
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index ce471e033774..13c88dfd790d 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -630,14 +630,15 @@ int f2fs_get_node_info(struct f2fs_sb_info *sbi, nid_t 
nid,
         node_info_from_raw_nat(ni, &ne);
         f2fs_folio_put(folio, true);
  sanity_check:
-       if (__is_valid_data_blkaddr(ni->blk_addr) &&
+       if (unlikely(ni->nid != nid ||
Hi Chao,
(ni->nid==nid) should be always true? because the code:

ni->flag = 0;
ni->nid = nid;
retry:
or am I missing something?

Zhiguo,

Oh, I may missed something, let's ignore this patch.


+               (__is_valid_data_blkaddr(ni->blk_addr) &&
btw, Is it possible to detect that some valid Nid entries contain
incorrect content?
such as  ino/block_addr=NULL_ADDR in nid=4 entry?

Something like this?

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 65ca1a5eaa88..c458df92bb0d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4928,16 +4928,16 @@ static inline bool is_journalled_quota(struct 
f2fs_sb_info *sbi)
         return false;
  }

-static inline bool f2fs_quota_file(struct inode *inode)
+static inline bool f2fs_quota_file(struct f2fs_sb_info *sbi, nid_t ino)
  {
  #ifdef CONFIG_QUOTA
         int i;

-       if (!f2fs_sb_has_quota_ino(F2FS_I_SB(inode)))
+       if (!f2fs_sb_has_quota_ino(sbi))
                 return false;

         for (i = 0; i < MAXQUOTAS; i++) {
-               if (f2fs_qf_ino(F2FS_I_SB(inode)->sb, i) == inode->i_ino)
+               if (f2fs_qf_ino(sbi->sb, i) == ino)
                         return true;
         }
  #endif
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 921fb02c0f49..d1270b25ad7d 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -621,7 +621,7 @@ make_now:
                 inode->i_fop = &f2fs_file_operations;
                 inode->i_mapping->a_ops = &f2fs_dblock_aops;
                 if (IS_IMMUTABLE(inode) && !f2fs_compressed_file(inode) &&
-                   !f2fs_quota_file(inode))
+                   !f2fs_quota_file(sbi, inode->i_ino))
                         mapping_set_folio_min_order(inode->i_mapping, 0);
         } else if (S_ISDIR(inode->i_mode)) {
                 inode->i_op = &f2fs_dir_inode_operations;
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 7feead595ba5..10448e115ea0 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -643,6 +643,17 @@ sanity_check:
                 return -EFSCORRUPTED;
         }

Hi Chao
+       if (unlikely(f2fs_quota_file(sbi, ni->nid) &&
+               __is_valid_data_blkaddr(ni->blk_addr))) {
  __is_valid_data_blkaddr(ni->blk_addr) --> !
__is_valid_data_blkaddr(ni->blk_addr)??

Zhiguo,

Oh, yes.

+               set_sbi_flag(sbi, SBI_NEED_FSCK);
+               f2fs_err_ratelimited(sbi,
+                       "f2fs_get_node_info of %pS: inconsistent nat entry from 
qf_ino, "
+                       "ino:%u, nid:%u, blkaddr:%u, ver:%u, flag:%u",
+                       __builtin_return_address(0),
+                       ni->ino, ni->nid, ni->blk_addr, ni->version, ni->flag);
+               f2fs_handle_error(sbi, ERROR_INCONSISTENT_NAT);
+       }
+
I think this is ok for quota file,
and This is not easy to apply to all common cases( nid entry not only
for quota), right? ^^

Yes, I guess partial of them may be common case, which may happen in race
condition, e.g. truncate vs read.
Hi Chao,
Thanks for this explaination, so
Could you resend this official patch?

Done. :)

Thanks,

Thanks!
Thanks,

Thanks!
         /* cache nat entry */
         if (need_cache)
                 cache_nat_entry(sbi, nid, &ne);

Thanks,

Thanks
                 !f2fs_is_valid_blkaddr(sbi, ni->blk_addr,
-                                       DATA_GENERIC_ENHANCE)) {
+                                       DATA_GENERIC_ENHANCE)))) {
                 set_sbi_flag(sbi, SBI_NEED_FSCK);
                 f2fs_err_ratelimited(sbi,
-                       "f2fs_get_node_info of %pS: inconsistent nat entry, "
+                       "f2fs_get_node_info of %pS: nid:%u, inconsistent nat entry, 
"
                         "ino:%u, nid:%u, blkaddr:%u, ver:%u, flag:%u",
-                       __builtin_return_address(0),
+                       __builtin_return_address(0), nid,
                         ni->ino, ni->nid, ni->blk_addr, ni->version, ni->flag);
                 f2fs_handle_error(sbi, ERROR_INCONSISTENT_NAT);
                 return -EFSCORRUPTED;
--
2.49.0



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel





_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to