fs/f2fs/node.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
In order to avoid loading corrupted nat entry from disk.
Cc: stable@kernel.org
Signed-off-by: Chao Yu <chao@kernel.org>
---
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 ||
+ (__is_valid_data_blkaddr(ni->blk_addr) &&
!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.0Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
于2025年12月15日周一 20:34写道：
>
> In order to avoid loading corrupted nat entry from disk.
>
> Cc: stable@kernel.org
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> 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?
> + (__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?
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
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 12/16/25 09:36, Zhiguo Niu wrote:
> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> 于2025年12月15日周一 20:34写道：
>>
>> In order to avoid loading corrupted nat entry from disk.
>>
>> Cc: stable@kernel.org
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>> 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;
}
+ if (unlikely(f2fs_quota_file(sbi, ni->nid) &&
+ __is_valid_data_blkaddr(ni->blk_addr))) {
+ 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);
+ }
+
/* 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
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Chao Yu <chao@kernel.org> 于2025年12月16日周二 16:49写道：
>
> On 12/16/25 09:36, Zhiguo Niu wrote:
> > Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> > 于2025年12月15日周一 20:34写道：
> >>
> >> In order to avoid loading corrupted nat entry from disk.
> >>
> >> Cc: stable@kernel.org
> >> Signed-off-by: Chao Yu <chao@kernel.org>
> >> ---
> >> 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)??
> + 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? ^^
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
> >> Linux-f2fs-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
On 12/17/25 09:46, Zhiguo Niu wrote:
> Chao Yu <chao@kernel.org> 于2025年12月16日周二 16:49写道：
>>
>> On 12/16/25 09:36, Zhiguo Niu wrote:
>>> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
>>> 于2025年12月15日周一 20:34写道：
>>>>
>>>> In order to avoid loading corrupted nat entry from disk.
>>>>
>>>> Cc: stable@kernel.org
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> ---
>>>> 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.
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
>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>
Chao Yu <chao@kernel.org> 于2025年12月17日周三 16:03写道：
>
> On 12/17/25 09:46, Zhiguo Niu wrote:
> > Chao Yu <chao@kernel.org> 于2025年12月16日周二 16:49写道：
> >>
> >> On 12/16/25 09:36, Zhiguo Niu wrote:
> >>> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> >>> 于2025年12月15日周一 20:34写道：
> >>>>
> >>>> In order to avoid loading corrupted nat entry from disk.
> >>>>
> >>>> Cc: stable@kernel.org
> >>>> Signed-off-by: Chao Yu <chao@kernel.org>
> >>>> ---
> >>>> 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?
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
> >>>> Linux-f2fs-devel@lists.sourceforge.net
> >>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >>
>
On 12/18/2025 9:30 AM, Zhiguo Niu wrote:
> Chao Yu <chao@kernel.org> 于2025年12月17日周三 16:03写道：
>>
>> On 12/17/25 09:46, Zhiguo Niu wrote:
>>> Chao Yu <chao@kernel.org> 于2025年12月16日周二 16:49写道：
>>>>
>>>> On 12/16/25 09:36, Zhiguo Niu wrote:
>>>>> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
>>>>> 于2025年12月15日周一 20:34写道：
>>>>>>
>>>>>> In order to avoid loading corrupted nat entry from disk.
>>>>>>
>>>>>> Cc: stable@kernel.org
>>>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>>>> ---
>>>>>> 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
>>>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>>
>>
© 2016 - 2026 Red Hat, Inc.