* [f2fs-dev] [PATCH v3 0/2] f2fs: Add sanity checks before unlinking and loading inodes
@ 2025-11-04 8:46 Nikola Z. Ivanov
2025-11-04 8:46 ` [f2fs-dev] [PATCH v3 1/2] f2fs: Rename f2fs_unlink exit label Nikola Z. Ivanov
2025-11-04 8:46 ` [f2fs-dev] [PATCH v3 2/2] f2fs: Add sanity checks before unlinking and loading inodes Nikola Z. Ivanov
0 siblings, 2 replies; 4+ messages in thread
From: Nikola Z. Ivanov @ 2025-11-04 8:46 UTC (permalink / raw)
To: jaegeuk, chao, linux-f2fs-devel
Cc: david.hunter.linux, linux-kernel, khalid, skhan,
linux-kernel-mentees, Nikola Z. Ivanov
This series is provoked by syzbot warnings caused by corrupted directory
inode with i_nlink == 1 that passes the initial sanity check which will
only mark the filesystem as corrupted in case i_nlink == 0.
Tests:
- fio/fsmark parallel create/unlink on VM with f2fs root filesystem.
- syzbot
Changelog:
Changes from v1:
- Rename exit label in f2fs_unlink().
- Add sanity check in sanity_check_inode() and remove it from f2fs_iget()
as suggested by Chao Yu in order to detect on-disk corruption early.
https://lore.kernel.org/linux-f2fs-devel/d4b7c03c-6554-4407-b823-aecfcdf7dc3f@kernel.org/T/#t
Changes from v2:
- Remove i_nlink == 0 check from sanity_check_inode.
- Wrap i_nlink == 1 in unlikely() marco.
https://lore.kernel.org/linux-f2fs-devel/1f519357-a489-41fe-8159-a8e319aedd17@kernel.org/T/#u
Nikola Z. Ivanov (2):
f2fs: Rename f2fs_unlink exit label
f2fs: Add sanity checks before unlinking and loading inodes
fs/f2fs/inode.c | 6 ++++++
fs/f2fs/namei.c | 27 +++++++++++++++++----------
2 files changed, 23 insertions(+), 10 deletions(-)
--
2.51.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* [f2fs-dev] [PATCH v3 1/2] f2fs: Rename f2fs_unlink exit label 2025-11-04 8:46 [f2fs-dev] [PATCH v3 0/2] f2fs: Add sanity checks before unlinking and loading inodes Nikola Z. Ivanov @ 2025-11-04 8:46 ` Nikola Z. Ivanov 2025-11-04 8:46 ` [f2fs-dev] [PATCH v3 2/2] f2fs: Add sanity checks before unlinking and loading inodes Nikola Z. Ivanov 1 sibling, 0 replies; 4+ messages in thread From: Nikola Z. Ivanov @ 2025-11-04 8:46 UTC (permalink / raw) To: jaegeuk, chao, linux-f2fs-devel Cc: david.hunter.linux, linux-kernel, khalid, skhan, linux-kernel-mentees, Nikola Z. Ivanov Rename "fail" label to "out" as it's used as a default exit path out of f2fs_unlink as well as error path. Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com> --- fs/f2fs/namei.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index b882771e4699..40cf80fd9d9a 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -552,21 +552,21 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) if (unlikely(f2fs_cp_error(sbi))) { err = -EIO; - goto fail; + goto out; } err = f2fs_dquot_initialize(dir); if (err) - goto fail; + goto out; err = f2fs_dquot_initialize(inode); if (err) - goto fail; + goto out; de = f2fs_find_entry(dir, &dentry->d_name, &folio); if (!de) { if (IS_ERR(folio)) err = PTR_ERR(folio); - goto fail; + goto out; } if (unlikely(inode->i_nlink == 0)) { @@ -575,7 +575,7 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) err = -EFSCORRUPTED; set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK); f2fs_folio_put(folio, false); - goto fail; + goto out; } f2fs_balance_fs(sbi, true); @@ -585,7 +585,7 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) if (err) { f2fs_unlock_op(sbi); f2fs_folio_put(folio, false); - goto fail; + goto out; } f2fs_delete_entry(de, folio, dir, inode); f2fs_unlock_op(sbi); @@ -601,7 +601,7 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) if (IS_DIRSYNC(dir)) f2fs_sync_fs(sbi->sb, 1); -fail: +out: trace_f2fs_unlink_exit(inode, err); return err; } -- 2.51.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [f2fs-dev] [PATCH v3 2/2] f2fs: Add sanity checks before unlinking and loading inodes 2025-11-04 8:46 [f2fs-dev] [PATCH v3 0/2] f2fs: Add sanity checks before unlinking and loading inodes Nikola Z. Ivanov 2025-11-04 8:46 ` [f2fs-dev] [PATCH v3 1/2] f2fs: Rename f2fs_unlink exit label Nikola Z. Ivanov @ 2025-11-04 8:46 ` Nikola Z. Ivanov 2025-11-05 7:27 ` Chao Yu via Linux-f2fs-devel 1 sibling, 1 reply; 4+ messages in thread From: Nikola Z. Ivanov @ 2025-11-04 8:46 UTC (permalink / raw) To: jaegeuk, chao, linux-f2fs-devel Cc: david.hunter.linux, syzbot+c07d47c7bc68f47b9083, linux-kernel, khalid, skhan, linux-kernel-mentees, Nikola Z. Ivanov Add check for inode->i_nlink == 1 for directories during unlink, as their value is decremented twice, which can trigger a warning in drop_nlink. In such case mark the filesystem as corrupted and return from the function call with the relevant failure return value. Additionally add the check for i_nlink == 1 in sanity_check_inode in order to detect on-disk corruption early. Reported-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=c07d47c7bc68f47b9083 Tested-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com> --- fs/f2fs/inode.c | 6 ++++++ fs/f2fs/namei.c | 15 +++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index 8c4eafe9ffac..b808e1dc2ae7 100644 --- a/fs/f2fs/inode.c +++ b/fs/f2fs/inode.c @@ -294,6 +294,12 @@ static bool sanity_check_inode(struct inode *inode, struct folio *node_folio) return false; } + if (S_ISDIR(inode->i_mode) && unlikely(inode->i_nlink == 1)) { + f2fs_warn(F2FS_I_SB(inode), "%s: directory inode (ino=%lx) has a single i_nlink", + __func__, inode->i_ino); + return false; + } + if (f2fs_has_extra_attr(inode)) { if (!f2fs_sb_has_extra_attr(sbi)) { f2fs_warn(sbi, "%s: inode (ino=%lx) is with extra_attr, but extra_attr feature is off", diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index 40cf80fd9d9a..65af1d56a99e 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -572,10 +572,11 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) if (unlikely(inode->i_nlink == 0)) { f2fs_warn(F2FS_I_SB(inode), "%s: inode (ino=%lx) has zero i_nlink", __func__, inode->i_ino); - err = -EFSCORRUPTED; - set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK); - f2fs_folio_put(folio, false); - goto out; + goto corrupted; + } else if (S_ISDIR(inode->i_mode) && unlikely(inode->i_nlink == 1)) { + f2fs_warn(F2FS_I_SB(inode), "%s: directory inode (ino=%lx) has a single i_nlink", + __func__, inode->i_ino); + goto corrupted; } f2fs_balance_fs(sbi, true); @@ -601,6 +602,12 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) if (IS_DIRSYNC(dir)) f2fs_sync_fs(sbi->sb, 1); + + goto out; +corrupted: + err = -EFSCORRUPTED; + set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK); + f2fs_folio_put(folio, false); out: trace_f2fs_unlink_exit(inode, err); return err; -- 2.51.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH v3 2/2] f2fs: Add sanity checks before unlinking and loading inodes 2025-11-04 8:46 ` [f2fs-dev] [PATCH v3 2/2] f2fs: Add sanity checks before unlinking and loading inodes Nikola Z. Ivanov @ 2025-11-05 7:27 ` Chao Yu via Linux-f2fs-devel 0 siblings, 0 replies; 4+ messages in thread From: Chao Yu via Linux-f2fs-devel @ 2025-11-05 7:27 UTC (permalink / raw) To: Nikola Z. Ivanov, jaegeuk, linux-f2fs-devel Cc: david.hunter.linux, syzbot+c07d47c7bc68f47b9083, linux-kernel, khalid, skhan, linux-kernel-mentees On 11/4/25 16:46, Nikola Z. Ivanov wrote: > Add check for inode->i_nlink == 1 for directories during unlink, > as their value is decremented twice, which can trigger a warning in > drop_nlink. In such case mark the filesystem as corrupted and return > from the function call with the relevant failure return value. > > Additionally add the check for i_nlink == 1 in > sanity_check_inode in order to detect on-disk corruption early. > Cc: stable@kernel.org > Reported-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=c07d47c7bc68f47b9083 > Tested-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com > Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com> > --- > fs/f2fs/inode.c | 6 ++++++ > fs/f2fs/namei.c | 15 +++++++++++---- > 2 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > index 8c4eafe9ffac..b808e1dc2ae7 100644 > --- a/fs/f2fs/inode.c > +++ b/fs/f2fs/inode.c > @@ -294,6 +294,12 @@ static bool sanity_check_inode(struct inode *inode, struct folio *node_folio) > return false; > } > > + if (S_ISDIR(inode->i_mode) && unlikely(inode->i_nlink == 1)) { > + f2fs_warn(F2FS_I_SB(inode), "%s: directory inode (ino=%lx) has a single i_nlink", s/F2FS_I_SB(inode)/sbi > + __func__, inode->i_ino); > + return false; > + } > + > if (f2fs_has_extra_attr(inode)) { > if (!f2fs_sb_has_extra_attr(sbi)) { > f2fs_warn(sbi, "%s: inode (ino=%lx) is with extra_attr, but extra_attr feature is off", > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > index 40cf80fd9d9a..65af1d56a99e 100644 > --- a/fs/f2fs/namei.c > +++ b/fs/f2fs/namei.c > @@ -572,10 +572,11 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) > if (unlikely(inode->i_nlink == 0)) { > f2fs_warn(F2FS_I_SB(inode), "%s: inode (ino=%lx) has zero i_nlink", > __func__, inode->i_ino); > - err = -EFSCORRUPTED; > - set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK); > - f2fs_folio_put(folio, false); > - goto out; > + goto corrupted; > + } else if (S_ISDIR(inode->i_mode) && unlikely(inode->i_nlink == 1)) { > + f2fs_warn(F2FS_I_SB(inode), "%s: directory inode (ino=%lx) has a single i_nlink", Ditto, > + __func__, inode->i_ino); > + goto corrupted; > } > > f2fs_balance_fs(sbi, true); > @@ -601,6 +602,12 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) > > if (IS_DIRSYNC(dir)) > f2fs_sync_fs(sbi->sb, 1); > + > + goto out; > +corrupted: > + err = -EFSCORRUPTED; > + set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK); Ditto, Thanks, > + f2fs_folio_put(folio, false); > out: > trace_f2fs_unlink_exit(inode, err); > return err; _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-05 7:28 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-04 8:46 [f2fs-dev] [PATCH v3 0/2] f2fs: Add sanity checks before unlinking and loading inodes Nikola Z. Ivanov 2025-11-04 8:46 ` [f2fs-dev] [PATCH v3 1/2] f2fs: Rename f2fs_unlink exit label Nikola Z. Ivanov 2025-11-04 8:46 ` [f2fs-dev] [PATCH v3 2/2] f2fs: Add sanity checks before unlinking and loading inodes Nikola Z. Ivanov 2025-11-05 7:27 ` Chao Yu via Linux-f2fs-devel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).