[PATCH v2] f2fs: evict: truncate page cache before clear_inode

Taerang Kim posted 1 patch 1 month, 3 weeks ago
fs/f2fs/inode.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH v2] f2fs: evict: truncate page cache before clear_inode
Posted by Taerang Kim 1 month, 3 weeks ago
syzbot reports a BUG_ON(inode->i_data.nrpages) in clear_inode() when
mounting a corrupted f2fs image.

I agree with Dmitry's RFC that dropping page #0 in f2fs_truncate()
can address this reproducer, since f2fs_convert_inline_inode() may
grab page #0 via f2fs_grab_cache_folio() and leave it cached on the
clear_out success path.

However, clear_inode() requires the inode mapping to be empty, and it is
hard to guarantee that the page cache can only be populated from this
truncate/inline-conversion path. Make f2fs_evict_inode() defensively
truncate any remaining page cache before calling clear_inode(), so
nrpages is guaranteed to be 0 regardless of how the cache was populated.

Link: https://lore.kernel.org/linux-f2fs-devel/20260206092958.578191-1-dmantipov@yandex.ru/
Reported-by: syzbot+fc026e87558558f75c00@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fc026e87558558f75c00
Signed-off-by: Taerang Kim <kth5965@gmail.com>
---
v2: rebase onto torvalds/master (fsverity_cleanup_inode() not present);
    fix patch context, no functional change.

 fs/f2fs/inode.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index e0f850b3f0c3..e7942e6e312c 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -1006,6 +1006,13 @@ void f2fs_evict_inode(struct inode *inode)
 	}
 out_clear:
 	fscrypt_put_encryption_info(inode);
+	/*
+	 * Defensively truncate any remaining page cache, e.g.
+	 * f2fs_convert_inline_inode() called from f2fs_truncate()
+	 * may leave page #0 behind in the page cache when the
+	 * inline conversion takes the clear_out success path.
+	 */
+	truncate_inode_pages_final(&inode->i_data);
 	clear_inode(inode);
 }
 
-- 
2.43.0
Re: [PATCH v2] f2fs: evict: truncate page cache before clear_inode
Posted by Chao Yu 1 month, 1 week ago
On 2/25/26 00:54, Taerang Kim wrote:
> syzbot reports a BUG_ON(inode->i_data.nrpages) in clear_inode() when
> mounting a corrupted f2fs image.
> 
> I agree with Dmitry's RFC that dropping page #0 in f2fs_truncate()
> can address this reproducer, since f2fs_convert_inline_inode() may
> grab page #0 via f2fs_grab_cache_folio() and leave it cached on the
> clear_out success path.

I suspect that we may miss some corner cases in f2fs_evict_inode() ->
f2fs_truncate(), can we figure out the root cause of this issue first
rather than just truncating all page cache before clear_inode()?
Otherwise, current fix may cover potential bug.

Thanks,

> 
> However, clear_inode() requires the inode mapping to be empty, and it is
> hard to guarantee that the page cache can only be populated from this
> truncate/inline-conversion path. Make f2fs_evict_inode() defensively
> truncate any remaining page cache before calling clear_inode(), so
> nrpages is guaranteed to be 0 regardless of how the cache was populated.
> 
> Link: https://lore.kernel.org/linux-f2fs-devel/20260206092958.578191-1-dmantipov@yandex.ru/
> Reported-by: syzbot+fc026e87558558f75c00@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=fc026e87558558f75c00
> Signed-off-by: Taerang Kim <kth5965@gmail.com>
> ---
> v2: rebase onto torvalds/master (fsverity_cleanup_inode() not present);
>     fix patch context, no functional change.
> 
>  fs/f2fs/inode.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index e0f850b3f0c3..e7942e6e312c 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -1006,6 +1006,13 @@ void f2fs_evict_inode(struct inode *inode)
>  	}
>  out_clear:
>  	fscrypt_put_encryption_info(inode);
> +	/*
> +	 * Defensively truncate any remaining page cache, e.g.
> +	 * f2fs_convert_inline_inode() called from f2fs_truncate()
> +	 * may leave page #0 behind in the page cache when the
> +	 * inline conversion takes the clear_out success path.
> +	 */
> +	truncate_inode_pages_final(&inode->i_data);
>  	clear_inode(inode);
>  }
>
Re: [f2fs-dev] [PATCH v2] f2fs: evict: truncate page cache before clear_inode
Posted by kth5965@gmail.com 1 month ago
Hi Chao,

I checked the repro again based on your comment and added some debug
logs around the related paths.

What I saw was roughly as follows.

There was already an abnormal inline state in the read path:
inline flag: set
data_exist: clear
blocks: present

This case was not rejected by sanity_check_inode(). From what I saw,
the inline sanity check does an early return when inode_has_blocks()
is true, so I think this case was skipped there. I think this may also
explain why there was no sanity warning in the log.

After that, in the eviction path, i_size was already reduced to 0, but
f2fs_truncate() still entered the inline conversion path, and
f2fs_convert_inline_inode() created folio 0 in the page cache first.

Then f2fs_convert_inline_folio() handled the empty inline case as
success because of !f2fs_exist_data(inode), and the created folio 0
remained in the page cache. Because of this, nrpages stayed 1 right
before clear_inode().

From this, I think there may be two possible directions for fixing
this:

1. prevent folio 0 from being created at all in the empty inline case,
   or delay folio creation until it is actually needed
2. detect or guard this abnormal inline state earlier, in sanity check
   or before that stage

At this point, both directions seem possible to me. I wanted to ask
which direction you think would be more appropriate.

If there is anything else I should check, please let me know.

Thanks.
Re: [f2fs-dev] [PATCH v2] f2fs: evict: truncate page cache before clear_inode
Posted by Chao Yu 4 weeks, 1 day ago
Hi,

On 2026/3/17 23:28, kth5965@gmail.com wrote:
> Hi Chao,
> 
> I checked the repro again based on your comment and added some debug
> logs around the related paths.
> 
> What I saw was roughly as follows.
> 
> There was already an abnormal inline state in the read path:
> inline flag: set
> data_exist: clear
> blocks: present
> 
> This case was not rejected by sanity_check_inode(). From what I saw,
> the inline sanity check does an early return when inode_has_blocks()
> is true, so I think this case was skipped there. I think this may also
> explain why there was no sanity warning in the log.
> 
> After that, in the eviction path, i_size was already reduced to 0, but
> f2fs_truncate() still entered the inline conversion path, and
> f2fs_convert_inline_inode() created folio 0 in the page cache first.
> 
> Then f2fs_convert_inline_folio() handled the empty inline case as
> success because of !f2fs_exist_data(inode), and the created folio 0
> remained in the page cache. Because of this, nrpages stayed 1 right
> before clear_inode().

The root cause that you pointing out make sense to me. Thanks for the debug
and analysis!

> 
>  From this, I think there may be two possible directions for fixing
> this:
> 
> 1. prevent folio 0 from being created at all in the empty inline case,
>     or delay folio creation until it is actually needed
> 2. detect or guard this abnormal inline state earlier, in sanity check
>     or before that stage

IMO, I think we need to fix both two places:

2) helps to detect this in early stage, so that we can know target inode is
corrupted via log and error number returned.

However, during 2) process, we will still call into f2fs_evict_inode(), and
do inline conversion eventually, so we need to implement 1) to avoid remained
page #0 cache, IIUC.

What do you think?

Thanks,

> 
> At this point, both directions seem possible to me. I wanted to ask
> which direction you think would be more appropriate.
> 
> If there is anything else I should check, please let me know.
> 
> Thanks.
Re: [f2fs-dev] [PATCH v2] f2fs: evict: truncate page cache before clear_inode
Posted by kth5965@gmail.com 4 weeks ago
Hi Chao,

Yes, I agree with your point.

I also think 2) is useful to detect the corrupted inode earlier and to
make the issue visible through log and returned error.

But 2) alone does not seem sufficient, because we can still reach inode
cleanup later, and then `f2fs_evict_inode()` can still go through the
inline conversion path.

So it makes sense to me that we need both:

1. earlier detection / guarding in sanity check
2. fixing the empty inline conversion path so page #0 cache is not left
   behind

If this understanding is correct, I will prepare the fix in that
direction.

Thanks.
Re: [f2fs-dev] [PATCH v2] f2fs: evict: truncate page cache before clear_inode
Posted by Chao Yu 3 weeks, 3 days ago
On 3/19/26 23:11, kth5965@gmail.com wrote:
> Hi Chao,
> 
> Yes, I agree with your point.
> 
> I also think 2) is useful to detect the corrupted inode earlier and to
> make the issue visible through log and returned error.
> 
> But 2) alone does not seem sufficient, because we can still reach inode
> cleanup later, and then `f2fs_evict_inode()` can still go through the
> inline conversion path.
> 
> So it makes sense to me that we need both:
> 
> 1. earlier detection / guarding in sanity check
> 2. fixing the empty inline conversion path so page #0 cache is not left
>    behind
> 
> If this understanding is correct, I will prepare the fix in that
> direction.

Please go ahead w/ this, thank you!

Thanks,

> 
> Thanks.
Re: [f2fs-dev] [PATCH v2] f2fs: evict: truncate page cache before clear_inode
Posted by kth5965@gmail.com 2 weeks, 2 days ago
Sorry for the late reply.

I worked in that direction, and the fix ended up touching a bit more than I first expected.

One part that grew a bit was the empty/no-data inline conversion side.
To handle that case cleanly, I ended up consolidating part of the inline-state teardown so the no-data path and the normal inline-conversion cleanup follow the same path.
This also lets f2fs_convert_inline_inode() avoid grabbing folio #0 in the empty inline case.

With the current version, the reproducer no longer reaches the late clear_inode() BUG and instead fails earlier with Failed to read root inode.

If you think that cleanup change should be trimmed or split out, please let me know.

The current diff is below.

diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 0a1052d5ee62..89e8a0fbb923 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -53,8 +53,8 @@ bool f2fs_sanity_check_inline_data(struct inode *inode, struct folio *ifolio)
	if (!f2fs_has_inline_data(inode))
		return false;

-	if (inode_has_blocks(inode, ifolio))
-		return false;
+	if (!f2fs_exist_data(inode) && inode_has_blocks(inode, ifolio))
+		return true;

	if (!support_inline_data(inode))
		return true;
@@ -142,6 +142,17 @@ int f2fs_read_inline_data(struct inode *inode, struct folio *folio)
	return 0;
 }

+static void f2fs_clear_inline_inode(struct dnode_of_data *dn)
+{
+	f2fs_folio_wait_writeback(dn->inode_folio, NODE, true, true);
+	clear_inode_flag(dn->inode, FI_DATA_EXIST);
+	stat_dec_inline_inode(dn->inode);
+	clear_inode_flag(dn->inode, FI_INLINE_DATA);
+	set_raw_inline(dn->inode, F2FS_INODE(dn->inode_folio));
+	folio_mark_dirty(dn->inode_folio);
+	folio_clear_f2fs_inline(dn->inode_folio);
+}
+
 int f2fs_convert_inline_folio(struct dnode_of_data *dn, struct folio *folio)
 {
	struct f2fs_io_info fio = {
@@ -157,8 +168,10 @@ int f2fs_convert_inline_folio(struct dnode_of_data *dn, struct folio *folio)
	struct node_info ni;
	int dirty, err;

-	if (!f2fs_exist_data(dn->inode))
-		goto clear_out;
+	if (!f2fs_exist_data(dn->inode)) {
+		f2fs_clear_inline_inode(dn);
+		goto out;
+	}

	err = f2fs_reserve_block(dn, 0);
	if (err)
		return err;
@@ -206,10 +219,8 @@ int f2fs_convert_inline_folio(struct dnode_of_data *dn, struct folio *folio)

	/* clear inline data and flag after data writeback */
	f2fs_truncate_inline_inode(dn->inode, dn->inode_folio, 0);
-	folio_clear_f2fs_inline(dn->inode_folio);
-clear_out:
-	stat_dec_inline_inode(dn->inode);
-	clear_inode_flag(dn->inode, FI_INLINE_DATA);
+	f2fs_clear_inline_inode(dn);
+out:
	f2fs_put_dnode(dn);
	return 0;
 }
@@ -219,7 +230,7 @@ int f2fs_convert_inline_inode(struct inode *inode)
	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
	struct dnode_of_data dn;
	struct f2fs_lock_context lc;
-	struct folio *ifolio, *folio;
+	struct folio *ifolio, *folio = NULL;
	int err = 0;

	if (f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb))
		return -EROFS;
@@ -232,9 +243,13 @@ int f2fs_convert_inline_inode(struct inode *inode)
	if (err)
		return err;

-	folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
-	if (IS_ERR(folio))
-		return PTR_ERR(folio);
+	if (f2fs_exist_data(inode)) {
+		folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
+		if (IS_ERR(folio))
+			return PTR_ERR(folio);
+	}
+
+	set_new_dnode(&dn, inode, NULL, NULL, 0);

	f2fs_lock_op(sbi, &lc);

@@ -249,11 +264,11 @@ int f2fs_convert_inline_inode(struct inode *inode)
	if (f2fs_has_inline_data(inode))
		err = f2fs_convert_inline_folio(&dn, folio);

-	f2fs_put_dnode(&dn);
 out:
+	f2fs_put_dnode(&dn);
	f2fs_unlock_op(sbi, &lc);
-
-	f2fs_folio_put(folio, true);
+	if (folio)
+		f2fs_folio_put(folio, true);

	if (!err)
		f2fs_balance_fs(sbi, dn.node_changed);
Re: [f2fs-dev] [PATCH v2] f2fs: evict: truncate page cache before clear_inode
Posted by kth5965@gmail.com 1 month, 1 week ago
Hi Chao,

Thanks for the comment.

I took another look at the path, and I think you are right that my
current fix is too broad and may hide the real problem.

It looks like this happens in a more specific case, where an inode
still has FI_INLINE_DATA set, but FI_DATA_EXIST is not set.

In that case, f2fs_truncate() goes into the inline conversion path,
and f2fs_convert_inline_inode() grabs folio 0 before it checks
whether there is real inline data to move.

Then f2fs_convert_inline_folio() does this:

	if (!f2fs_exist_data(dn->inode))
		goto clear_out;

So for the empty-inline case, it returns success, but folio 0 seems to
have already been added to the page cache by then.

From what I can see, f2fs_grab_cache_folio() may create folio 0 and add
it to inode->i_mapping when there is no folio at that index, so
inode->i_data.nrpages becomes 1 there.

After that, f2fs_folio_put() only drops the ref, and the folio stays
there. Because of that, clear_inode() later sees nrpages != 0 and
hits the BUG.

This is the flow I am seeing:

f2fs_evict_inode()
  -> truncate_inode_pages_final(&inode->i_data)
     // nrpages = 0

  -> i_size_write(inode, 0)
  -> f2fs_truncate(inode)
     -> !f2fs_may_inline_data(inode)
     -> f2fs_convert_inline_inode(inode)
        -> f2fs_grab_cache_folio(inode->i_mapping, 0, false)
           // folio 0 is inserted into page cache
           // nrpages = 1
        -> f2fs_convert_inline_folio(&dn, folio)
           -> !f2fs_exist_data(inode)
           -> clear_out
        -> f2fs_folio_put(folio, true)
           // only drops the ref
           // folio stays in page cache

  -> clear_inode()
     -> BUG_ON(inode->i_data.nrpages)

So it seems better to fix this in the inline conversion path,
instead of truncating all page cache again at the end of eviction.

If you agree, I can send a new patch in that direction.

Thanks,
Re: [f2fs-dev] [PATCH v2] f2fs: evict: truncate page cache before clear_inode
Posted by Chao Yu 1 month ago
On 3/9/2026 10:25 PM, kth5965@gmail.com wrote:
> Hi Chao,
> 
> Thanks for the comment.
> 
> I took another look at the path, and I think you are right that my
> current fix is too broad and may hide the real problem.
> 
> It looks like this happens in a more specific case, where an inode
> still has FI_INLINE_DATA set, but FI_DATA_EXIST is not set.
> 
> In that case, f2fs_truncate() goes into the inline conversion path,
> and f2fs_convert_inline_inode() grabs folio 0 before it checks
> whether there is real inline data to move.
> 
> Then f2fs_convert_inline_folio() does this:
> 
> 	if (!f2fs_exist_data(dn->inode))
> 		goto clear_out;
> 
> So for the empty-inline case, it returns success, but folio 0 seems to
> have already been added to the page cache by then.
> 
>  From what I can see, f2fs_grab_cache_folio() may create folio 0 and add
> it to inode->i_mapping when there is no folio at that index, so
> inode->i_data.nrpages becomes 1 there.
> 
> After that, f2fs_folio_put() only drops the ref, and the folio stays
> there. Because of that, clear_inode() later sees nrpages != 0 and
> hits the BUG.
> 
> This is the flow I am seeing:
> 
> f2fs_evict_inode()
>    -> truncate_inode_pages_final(&inode->i_data)
>       // nrpages = 0
> 
>    -> i_size_write(inode, 0)
>    -> f2fs_truncate(inode)
>       -> !f2fs_may_inline_data(inode)
>       -> f2fs_convert_inline_inode(inode)
>          -> f2fs_grab_cache_folio(inode->i_mapping, 0, false)
>             // folio 0 is inserted into page cache
>             // nrpages = 1
>          -> f2fs_convert_inline_folio(&dn, folio)
>             -> !f2fs_exist_data(inode)
>             -> clear_out
>          -> f2fs_folio_put(folio, true)
>             // only drops the ref
>             // folio stays in page cache
> 
>    -> clear_inode()
>       -> BUG_ON(inode->i_data.nrpages)

Hi,

Sorry for the late reply.

Yes, we may missed to truncate page #0 cache once we converted inline
inode.

The reason why we convert the inline inode is the inode may already
be corrupted (filesize is larger than MAX_INLINE_DATA(), and it has
FI_INLINE_DATA flag).

My question is: we should has ability to detect such corrupted inode
in sanity_check_inode(), however, I didn't find any log related to that,
any thoughts?

Thanks,

> 
> So it seems better to fix this in the inline conversion path,
> instead of truncating all page cache again at the end of eviction.
> 
> If you agree, I can send a new patch in that direction.
> 
> Thanks,