f2fs: Perform sanity check before unlinking directory inode

[PATCH] f2fs: Perform sanity check before unlinking directory inode

Posted by Nikola Z. Ivanov 2 months, 2 weeks ago

Current i_nlink corruption check does not take into account
directory inodes which have one additional i_nlink for their "." entry.

Add additional check and a common corruption path.

Reported-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c07d47c7bc68f47b9083
Fixes: 81edb983b3f5 ("f2fs: add check for deleted inode")
Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
---
 fs/f2fs/namei.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index b882771e4699..68b33e8089b0 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -502,12 +502,14 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
 		goto out;
 	}
 
-	if (inode->i_nlink == 0) {
+	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);
-		goto out_iput;
+		goto corrupted;
+	} else if (unlikely(S_ISDIR(inode->i_mode) && 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;
 	}
 
 	if (IS_ENCRYPTED(dir) &&
@@ -533,6 +535,9 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
 	trace_f2fs_lookup_end(dir, !IS_ERR_OR_NULL(new) ? new : dentry,
 				ino, IS_ERR(new) ? PTR_ERR(new) : err);
 	return new;
+corrupted:
+	err = -EFSCORRUPTED;
+	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
 out_iput:
 	iput(inode);
 out:
@@ -572,10 +577,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 fail;
+		goto corrupted;
+	} else if (unlikely(S_ISDIR(inode->i_mode) && 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 +607,12 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
 
 	if (IS_DIRSYNC(dir))
 		f2fs_sync_fs(sbi->sb, 1);
+
+	goto fail;
+corrupted:
+	err = -EFSCORRUPTED;
+	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
+	f2fs_folio_put(folio, false);
 fail:
 	trace_f2fs_unlink_exit(inode, err);
 	return err;
-- 
2.51.0

Re: [PATCH] f2fs: Perform sanity check before unlinking directory inode

Posted by Chao Yu 2 months, 1 week ago

On 10/3/2025 9:47 PM, Nikola Z. Ivanov wrote:
> Current i_nlink corruption check does not take into account
> directory inodes which have one additional i_nlink for their "." entry.
> 
> Add additional check and a common corruption path.
> 
> Reported-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c07d47c7bc68f47b9083
> Fixes: 81edb983b3f5 ("f2fs: add check for deleted inode")
> Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
> ---
>   fs/f2fs/namei.c | 28 ++++++++++++++++++++--------
>   1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index b882771e4699..68b33e8089b0 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -502,12 +502,14 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
>   		goto out;
>   	}
>   
> -	if (inode->i_nlink == 0) {
> +	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);
> -		goto out_iput;
> +		goto corrupted;
> +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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;

Can we detect such corruption in sanity_check_inode() as well? So that if
f2fs internal flow calls f2fs_iget() on corrupted inode, we can set SBI_NEED_FSCK
flag and then triggering fsck repairment later.

Thanks,

>   	}
>   
>   	if (IS_ENCRYPTED(dir) &&
> @@ -533,6 +535,9 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
>   	trace_f2fs_lookup_end(dir, !IS_ERR_OR_NULL(new) ? new : dentry,
>   				ino, IS_ERR(new) ? PTR_ERR(new) : err);
>   	return new;
> +corrupted:
> +	err = -EFSCORRUPTED;
> +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
>   out_iput:
>   	iput(inode);
>   out:
> @@ -572,10 +577,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 fail;
> +		goto corrupted;
> +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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 +607,12 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
>   
>   	if (IS_DIRSYNC(dir))
>   		f2fs_sync_fs(sbi->sb, 1);
> +
> +	goto fail;
> +corrupted:
> +	err = -EFSCORRUPTED;
> +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
> +	f2fs_folio_put(folio, false);
>   fail:
>   	trace_f2fs_unlink_exit(inode, err);
>   	return err;

Re: [PATCH] f2fs: Perform sanity check before unlinking directory inode

Posted by Nikola Z. Ivanov 2 months ago

On Thu, Oct 09, 2025 at 10:54:40AM +0800, Chao Yu wrote:
> On 10/3/2025 9:47 PM, Nikola Z. Ivanov wrote:
> > Current i_nlink corruption check does not take into account
> > directory inodes which have one additional i_nlink for their "." entry.
> > 
> > Add additional check and a common corruption path.
> > 
> > Reported-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=c07d47c7bc68f47b9083
> > Fixes: 81edb983b3f5 ("f2fs: add check for deleted inode")
> > Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
> > ---
> >   fs/f2fs/namei.c | 28 ++++++++++++++++++++--------
> >   1 file changed, 20 insertions(+), 8 deletions(-)
> > 
> > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> > index b882771e4699..68b33e8089b0 100644
> > --- a/fs/f2fs/namei.c
> > +++ b/fs/f2fs/namei.c
> > @@ -502,12 +502,14 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
> >   		goto out;
> >   	}
> > -	if (inode->i_nlink == 0) {
> > +	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);
> > -		goto out_iput;
> > +		goto corrupted;
> > +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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;
> 
> Can we detect such corruption in sanity_check_inode() as well? So that if
> f2fs internal flow calls f2fs_iget() on corrupted inode, we can set SBI_NEED_FSCK
> flag and then triggering fsck repairment later.
> 
> Thanks,
> 
> >   	}
> >   	if (IS_ENCRYPTED(dir) &&
> > @@ -533,6 +535,9 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
> >   	trace_f2fs_lookup_end(dir, !IS_ERR_OR_NULL(new) ? new : dentry,
> >   				ino, IS_ERR(new) ? PTR_ERR(new) : err);
> >   	return new;
> > +corrupted:
> > +	err = -EFSCORRUPTED;
> > +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
> >   out_iput:
> >   	iput(inode);
> >   out:
> > @@ -572,10 +577,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 fail;
> > +		goto corrupted;
> > +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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 +607,12 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
> >   	if (IS_DIRSYNC(dir))
> >   		f2fs_sync_fs(sbi->sb, 1);
> > +
> > +	goto fail;
> > +corrupted:
> > +	err = -EFSCORRUPTED;
> > +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
> > +	f2fs_folio_put(folio, false);
> >   fail:
> >   	trace_f2fs_unlink_exit(inode, err);
> >   	return err;
> 

Hi Chao,

Thank you for the suggestion.
I will add this to sanity_check_inode and remove it
from f2fs_lookup as it becomes redundant since f2fs_lookup
obtains the inode through f2fs_iget. For f2fs_unlink I will
move the i_nlink == 1 check to f2fs_rmdir.

I will send v2 as soon as I do some more testing.

Re: [PATCH] f2fs: Perform sanity check before unlinking directory inode

Posted by Chao Yu 2 months ago

On 10/13/25 05:19, Nikola Z. Ivanov wrote:
> On Thu, Oct 09, 2025 at 10:54:40AM +0800, Chao Yu wrote:
>> On 10/3/2025 9:47 PM, Nikola Z. Ivanov wrote:
>>> Current i_nlink corruption check does not take into account
>>> directory inodes which have one additional i_nlink for their "." entry.
>>>
>>> Add additional check and a common corruption path.
>>>
>>> Reported-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=c07d47c7bc68f47b9083
>>> Fixes: 81edb983b3f5 ("f2fs: add check for deleted inode")
>>> Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
>>> ---
>>>   fs/f2fs/namei.c | 28 ++++++++++++++++++++--------
>>>   1 file changed, 20 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>>> index b882771e4699..68b33e8089b0 100644
>>> --- a/fs/f2fs/namei.c
>>> +++ b/fs/f2fs/namei.c
>>> @@ -502,12 +502,14 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
>>>   		goto out;
>>>   	}
>>> -	if (inode->i_nlink == 0) {
>>> +	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);
>>> -		goto out_iput;
>>> +		goto corrupted;
>>> +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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;
>>
>> Can we detect such corruption in sanity_check_inode() as well? So that if
>> f2fs internal flow calls f2fs_iget() on corrupted inode, we can set SBI_NEED_FSCK
>> flag and then triggering fsck repairment later.
>>
>> Thanks,
>>
>>>   	}
>>>   	if (IS_ENCRYPTED(dir) &&
>>> @@ -533,6 +535,9 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
>>>   	trace_f2fs_lookup_end(dir, !IS_ERR_OR_NULL(new) ? new : dentry,
>>>   				ino, IS_ERR(new) ? PTR_ERR(new) : err);
>>>   	return new;
>>> +corrupted:
>>> +	err = -EFSCORRUPTED;
>>> +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
>>>   out_iput:
>>>   	iput(inode);
>>>   out:
>>> @@ -572,10 +577,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 fail;
>>> +		goto corrupted;
>>> +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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 +607,12 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
>>>   	if (IS_DIRSYNC(dir))
>>>   		f2fs_sync_fs(sbi->sb, 1);
>>> +
>>> +	goto fail;
>>> +corrupted:
>>> +	err = -EFSCORRUPTED;
>>> +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
>>> +	f2fs_folio_put(folio, false);
>>>   fail:
>>>   	trace_f2fs_unlink_exit(inode, err);
>>>   	return err;
>>
> 
> Hi Chao,
> 
> Thank you for the suggestion.
> I will add this to sanity_check_inode and remove it
> from f2fs_lookup as it becomes redundant since f2fs_lookup
> obtains the inode through f2fs_iget. For f2fs_unlink I will
> move the i_nlink == 1 check to f2fs_rmdir.

Hi Nikola,

I meant we can move the i_nlink == 1 check from both f2fs_lookup() and
f2fs_unlink() to sanity_check_inode(), because before we create in-memory
inode, we will always call sanity_check_inode().

Let me know if you have other concerns.

Thanks,

> 
> I will send v2 as soon as I do some more testing.

Re: [PATCH] f2fs: Perform sanity check before unlinking directory inode

Posted by Nikola Z. Ivanov 2 months ago

On Mon, Oct 13, 2025 at 08:53:04PM +0800, Chao Yu wrote:
> On 10/13/25 05:19, Nikola Z. Ivanov wrote:
> > On Thu, Oct 09, 2025 at 10:54:40AM +0800, Chao Yu wrote:
> >> On 10/3/2025 9:47 PM, Nikola Z. Ivanov wrote:
> >>> Current i_nlink corruption check does not take into account
> >>> directory inodes which have one additional i_nlink for their "." entry.
> >>>
> >>> Add additional check and a common corruption path.
> >>>
> >>> Reported-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com
> >>> Closes: https://syzkaller.appspot.com/bug?extid=c07d47c7bc68f47b9083
> >>> Fixes: 81edb983b3f5 ("f2fs: add check for deleted inode")
> >>> Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
> >>> ---
> >>>   fs/f2fs/namei.c | 28 ++++++++++++++++++++--------
> >>>   1 file changed, 20 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> >>> index b882771e4699..68b33e8089b0 100644
> >>> --- a/fs/f2fs/namei.c
> >>> +++ b/fs/f2fs/namei.c
> >>> @@ -502,12 +502,14 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
> >>>   		goto out;
> >>>   	}
> >>> -	if (inode->i_nlink == 0) {
> >>> +	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);
> >>> -		goto out_iput;
> >>> +		goto corrupted;
> >>> +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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;
> >>
> >> Can we detect such corruption in sanity_check_inode() as well? So that if
> >> f2fs internal flow calls f2fs_iget() on corrupted inode, we can set SBI_NEED_FSCK
> >> flag and then triggering fsck repairment later.
> >>
> >> Thanks,
> >>
> >>>   	}
> >>>   	if (IS_ENCRYPTED(dir) &&
> >>> @@ -533,6 +535,9 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
> >>>   	trace_f2fs_lookup_end(dir, !IS_ERR_OR_NULL(new) ? new : dentry,
> >>>   				ino, IS_ERR(new) ? PTR_ERR(new) : err);
> >>>   	return new;
> >>> +corrupted:
> >>> +	err = -EFSCORRUPTED;
> >>> +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
> >>>   out_iput:
> >>>   	iput(inode);
> >>>   out:
> >>> @@ -572,10 +577,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 fail;
> >>> +		goto corrupted;
> >>> +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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 +607,12 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
> >>>   	if (IS_DIRSYNC(dir))
> >>>   		f2fs_sync_fs(sbi->sb, 1);
> >>> +
> >>> +	goto fail;
> >>> +corrupted:
> >>> +	err = -EFSCORRUPTED;
> >>> +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
> >>> +	f2fs_folio_put(folio, false);
> >>>   fail:
> >>>   	trace_f2fs_unlink_exit(inode, err);
> >>>   	return err;
> >>
> > 
> > Hi Chao,
> > 
> > Thank you for the suggestion.
> > I will add this to sanity_check_inode and remove it
> > from f2fs_lookup as it becomes redundant since f2fs_lookup
> > obtains the inode through f2fs_iget. For f2fs_unlink I will
> > move the i_nlink == 1 check to f2fs_rmdir.
> 
> Hi Nikola,
> 
> I meant we can move the i_nlink == 1 check from both f2fs_lookup() and
> f2fs_unlink() to sanity_check_inode(), because before we create in-memory
> inode, we will always call sanity_check_inode().
> 
> Let me know if you have other concerns.
> 
> Thanks,
> 

The issue here is that sanity_check_inode will be called only when 
we initially read the inode off disk, not when it's already in the cache

The syzkaller repro does something like this:
Creates a directory structure /dir1/dir2 where dir1 has
i_nlink == 2, which is one less than it should. It then does
rmdir(/dir1/dir2) followed by rmdir(/dir1) which leads to the warning.

In such case what would you say should happen, should the second rmdir
fail and report the corruption, or do we close our eyes and just drop
i_nlink to 0 and possibly log a message that something isn't quite right?

Thank you,

> > 
> > I will send v2 as soon as I do some more testing.
>

Re: [PATCH] f2fs: Perform sanity check before unlinking directory inode

Posted by Shuah Khan 2 months, 1 week ago

On 10/3/25 07:47, Nikola Z. Ivanov wrote:
> Current i_nlink corruption check does not take into account
> directory inodes which have one additional i_nlink for their "." entry.
> 
> Add additional check and a common corruption path.

How does this change fix the problem? What is common corruption
path in this context?

> 
> Reported-by: syzbot+c07d47c7bc68f47b9083@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c07d47c7bc68f47b9083
> Fixes: 81edb983b3f5 ("f2fs: add check for deleted inode")
> Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>

How did you test this patch? Can add details?

> ---
>   fs/f2fs/namei.c | 28 ++++++++++++++++++++--------
>   1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index b882771e4699..68b33e8089b0 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -502,12 +502,14 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
>   		goto out;
>   	}
>   
> -	if (inode->i_nlink == 0) {
> +	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);
> -		goto out_iput;
> +		goto corrupted;
> +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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;
>   	}
>   
>   	if (IS_ENCRYPTED(dir) &&
> @@ -533,6 +535,9 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
>   	trace_f2fs_lookup_end(dir, !IS_ERR_OR_NULL(new) ? new : dentry,
>   				ino, IS_ERR(new) ? PTR_ERR(new) : err);
>   	return new;
> +corrupted:
> +	err = -EFSCORRUPTED;
> +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
>   out_iput:
>   	iput(inode);
>   out:
> @@ -572,10 +577,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 fail;
> +		goto corrupted;
> +	} else if (unlikely(S_ISDIR(inode->i_mode) && 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 +607,12 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
>   
>   	if (IS_DIRSYNC(dir))
>   		f2fs_sync_fs(sbi->sb, 1);
> +
> +	goto fail;
> +corrupted:
> +	err = -EFSCORRUPTED;
> +	set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
> +	f2fs_folio_put(folio, false);
>   fail:
>   	trace_f2fs_unlink_exit(inode, err);
>   	return err;