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

在 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