[PATCH] f2fs: fix ifolio leak in f2fs_move_inline_dirents

[PATCH] f2fs: fix ifolio leak in f2fs_move_inline_dirents

[Guanghui Yang]

f2fs_move_inline_dirents() documents that the caller grabs ifolio, and
that the function should release it on any error.

The f2fs_grab_cache_folio() failure path already drops ifolio, but the
f2fs_reserve_block() failure path only drops the newly grabbed folio at
the shared out label.  If f2fs_reserve_block() fails before clearing
dn.inode_folio, the caller's ifolio reference is left behind.

Release ifolio on this error path when dn.inode_folio is still set.

Signed-off-by: Guanghui Yang <3497809730@qq.com>
---
 fs/f2fs/inline.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index e2f7bedf1552..dea4ab957de8 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -427,8 +427,11 @@ static int f2fs_move_inline_dirents(struct inode *dir, struct folio *ifolio,
 
 	set_new_dnode(&dn, dir, ifolio, NULL, 0);
 	err = f2fs_reserve_block(&dn, 0);
-	if (err)
+	if (err) {
+		if (dn.inode_folio)
+			f2fs_folio_put(ifolio, true);
 		goto out;
+	}
 
 	if (unlikely(dn.data_blkaddr != NEW_ADDR)) {
 		f2fs_put_dnode(&dn);

base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
-- 
2.52.0.windows.1

Re: [PATCH] f2fs: fix ifolio leak in f2fs_move_inline_dirents

[Chao Yu]

On 7/13/26 14:40, Guanghui Yang wrote:
> f2fs_move_inline_dirents() documents that the caller grabs ifolio, and
> that the function should release it on any error.
> 
> The f2fs_grab_cache_folio() failure path already drops ifolio, but the
> f2fs_reserve_block() failure path only drops the newly grabbed folio at
> the shared out label.  If f2fs_reserve_block() fails before clearing
> dn.inode_folio, the caller's ifolio reference is left behind.
> 
> Release ifolio on this error path when dn.inode_folio is still set.
> 
> Signed-off-by: Guanghui Yang <3497809730@qq.com>
> ---
>   fs/f2fs/inline.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index e2f7bedf1552..dea4ab957de8 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -427,8 +427,11 @@ static int f2fs_move_inline_dirents(struct inode *dir, struct folio *ifolio,
>   
>   	set_new_dnode(&dn, dir, ifolio, NULL, 0);
>   	err = f2fs_reserve_block(&dn, 0);

f2fs_reserve_block() can handle the error case in itself?

Thanks,

> -	if (err)
> +	if (err) {
> +		if (dn.inode_folio)
> +			f2fs_folio_put(ifolio, true);
>   		goto out;
> +	}
>   
>   	if (unlikely(dn.data_blkaddr != NEW_ADDR)) {
>   		f2fs_put_dnode(&dn);
> 
> base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa

Re: [PATCH] f2fs: fix ifolio leak in f2fs_move_inline_dirents

[Guanghui Yang]

Thanks for taking a look.

f2fs_reserve_block() drops the dnode only when the inode folio was not
provided by the caller:

        bool need_put = dn->inode_folio ? false : true;
        ...
        if (err || need_put)
                f2fs_put_dnode(dn);

In this path, f2fs_move_inline_dirents() passes ifolio through
set_new_dnode(), so need_put is false and f2fs_reserve_block() keeps the
caller-provided folio.

The caller of do_convert_inline_dir() also drops ifolio only on success:

        err = do_convert_inline_dir(dir, ifolio, inline_dentry);
        if (!err)
                f2fs_folio_put(ifolio, true);

Therefore, when f2fs_reserve_block() fails, the ifolio reference still
appears to need cleanup in f2fs_move_inline_dirents(). I kept the cleanup
local to avoid changing f2fs_reserve_block() semantics for other callers.

Thanks,
Guanghui

Re: [PATCH] f2fs: fix ifolio leak in f2fs_move_inline_dirents

[Chao Yu]

On 7/19/26 17:05, Guanghui Yang wrote:
> Thanks for taking a look.
> 
> f2fs_reserve_block() drops the dnode only when the inode folio was not
> provided by the caller:
> 
>          bool need_put = dn->inode_folio ? false : true;
>          ...
>          if (err || need_put)

What about err is non-zero?

>                  f2fs_put_dnode(dn);
> 
> In this path, f2fs_move_inline_dirents() passes ifolio through
> set_new_dnode(), so need_put is false and f2fs_reserve_block() keeps the
> caller-provided folio.
> 
> The caller of do_convert_inline_dir() also drops ifolio only on success:
> 
>          err = do_convert_inline_dir(dir, ifolio, inline_dentry);
>          if (!err)
>                  f2fs_folio_put(ifolio, true);
> 
> Therefore, when f2fs_reserve_block() fails, the ifolio reference still
> appears to need cleanup in f2fs_move_inline_dirents(). I kept the cleanup
> local to avoid changing f2fs_reserve_block() semantics for other callers.
> 
> Thanks,
> Guanghui
> 

Re: [PATCH] f2fs: fix ifolio leak in f2fs_move_inline_dirents

[Guanghui Yang]

You are right. I misread the condition.

When err is non-zero, f2fs_put_dnode() is called regardless of need_put,
so dn.inode_folio is released on the f2fs_reserve_block() error path.
The caller only releases ifolio on success because the error cleanup has
already been handled here.

Please ignore this patch. Sorry for the confusion.

Thanks,
Guanghui