Re: [PATCH 2/2] nilfs: check new inode's nlink when rename

On Sat, Aug 8, 2026 at 5:31 PM wangjianjian (C) wrote:
>
> 在 2026/7/30 21:31, Ryusuke Konishi 写道:
> > On Thu, Jul 30, 2026 at 9:32 PM Wang Jianjian wrote:
> >>
> >> When rename, if new inode exists, then its nlink must not be zero.
> >>
> >> Signed-off-by: Wang Jianjian <[email protected]>
> >> ---
> >>   fs/nilfs2/namei.c | 5 +++++
> >>   1 file changed, 5 insertions(+)
> >>
> >> diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
> >> index e2fe95de3d71..db171789a316 100644
> >> --- a/fs/nilfs2/namei.c
> >> +++ b/fs/nilfs2/namei.c
> >> @@ -397,6 +397,11 @@ static int nilfs_rename(struct mnt_idmap *idmap,
> >>                  struct folio *new_folio;
> >>                  struct nilfs_dir_entry *new_de;
> >>
> >> +               if (new_inode->i_nlink == 0) {
> >> +                       err = -EFSCORRUPTED;
> >> +                       goto out_dir;
> >> +               }
> >> +
> >>                  err = -ENOTEMPTY;
> >>                  if (old_is_dir && !nilfs_empty_dir(new_inode))
> >>                          goto out_dir;
> >> --
> >> 2.34.1
> >>
> >
> > Wang Jianjian, regarding this patch - much like the previous one - is
> > this fix related to a Syzbot report?
> >
> > If so, could you please include the appropriate Reported-by and Closes tags?
> >
> > Also, please ensure that the filesystem name in the patch title is
> > "nilfs2" rather than "nilfs" (kernel patch title prefixes generally
> > follow the filesystem name, nilfs2).
> >
> > As for the patch itself: rather than using a stopgap measure to check
> > i_nlink inside nilfs_rename, could you investigate the root cause of
> > the inconsistency? If there is a flaw in the checks when reading the
> > disk inode or a race condition involving link count handling, that is
> > what should be resolved.
> >
> > Additionally, you are returning -EFSCORRUPTED for filesystem
> > corruption, but nilfs2 does not yet utilize this return code.
> > Following the established implementation pattern, when filesystem
> > corruption is detected, you should report it via nilfs_error() (which
> > logs an error message) and return -EIO (while I agree we should
> > eventually switch to -EFSCORRUPTED across the board, doing so here in
> > isolation would create inconsistency).
> >
>
> Hi Ryusuke,
> I have sent v2 patch and the root cause of the problem is in v2's commit
> message. I don't think this check is a stopgap, other filesystems, such
> like ext4, does this check too.
>
> nilfs_error will mark fs as readonly with default mount
> option(NILFS_MOUNT_ERRORS_RO), I am not sure if this is overkilled since
> this make the whole fs readonly(however, I add this in v2 patch).
> Perhaps we should isolate this error and only mark this file is bad ?
> For fs like ext4, we can mark one group to corrupted but nilfs seems not
> have this disk layout isolation.
> > Thanks,
> > Ryusuke Konishi
>
> --
> Regards

Hello Wang Jianjian,

Thanks for the submission.
I'm currently swamped with other reviews and testing, so please bear
with me for a bit while I work through them in order.

Thanks,
Ryusuke Konishi