| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHJ8P3Kgo+YesOqWr=-=HEBY2u8-r17q6HCzxA=qBWHweVH+mg@mail.gmail.com>
Date: Thu, 18 Dec 2025 09:30:24 +0800
From: Zhiguo Niu <niuzhiguo84@...il.com>
To: Chao Yu <chao@...nel.org>
Cc: jaegeuk@...nel.org, stable@...nel.org, linux-kernel@...r.kernel.org,
linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] f2fs: fix to sanity check on nat_entry.nid in f2fs_get_node_info()
Chao Yu <chao@...nel.org> 于2025年12月17日周三 16:03写道：
>
> On 12/17/25 09:46, Zhiguo Niu wrote:
> > Chao Yu <chao@...nel.org> 于2025年12月16日周二 16:49写道：
> >>
> >> On 12/16/25 09:36, Zhiguo Niu wrote:
> >>> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@...ts.sourceforge.net>
> >>> 于2025年12月15日周一 20:34写道：
> >>>>
> >>>> In order to avoid loading corrupted nat entry from disk.
> >>>>
> >>>> Cc: stable@...nel.org
> >>>> Signed-off-by: Chao Yu <chao@...nel.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@...ts.sourceforge.net
> >>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >>
>
Powered by blists - more mailing lists