[PATCH v2] f2fs: fix ifolio leak in f2fs_get_new_data_folio

[PATCH v2] f2fs: fix ifolio leak in f2fs_get_new_data_folio

[Guanghui Yang]

f2fs_get_new_data_folio() documents that ifolio is only set by
make_empty_dir(), and that ifolio should be released by this function on
any error.

The allocation failure path already follows this rule, but the
f2fs_reserve_block() failure path only drops the newly grabbed folio and
returns the error.  When make_empty_dir() passes a non-NULL ifolio, an
early f2fs_reserve_block() failure can leave the extra inode folio
reference held by the caller.

Release ifolio on this error path if f2fs_reserve_block() has not already
cleared dn.inode_folio.

Signed-off-by: Guanghui Yang <3497809730@qq.com>
---

Changes since v1:
- Check dn.inode_folio before releasing ifolio to avoid a double put when
  f2fs_reserve_block() has already cleared the dnode.

 fs/f2fs/data.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a765fda71536..6c573d392dac 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1477,6 +1477,8 @@ struct folio *f2fs_get_new_data_folio(struct inode *inode,
 	err = f2fs_reserve_block(&dn, index);
 	if (err) {
 		f2fs_folio_put(folio, true);
+		if (dn.inode_folio)
+			f2fs_folio_put(ifolio, true);
 		return ERR_PTR(err);
 	}
 	if (!ifolio)

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

Re: [PATCH v2] f2fs: fix ifolio leak in f2fs_get_new_data_folio

[Chao Yu]

On 7/13/26 14:16, Guanghui Yang wrote:
> f2fs_get_new_data_folio() documents that ifolio is only set by
> make_empty_dir(), and that ifolio should be released by this function on
> any error.
> 
> The allocation failure path already follows this rule, but the
> f2fs_reserve_block() failure path only drops the newly grabbed folio and
> returns the error.  When make_empty_dir() passes a non-NULL ifolio, an
> early f2fs_reserve_block() failure can leave the extra inode folio
> reference held by the caller.
> 
> Release ifolio on this error path if f2fs_reserve_block() has not already
> cleared dn.inode_folio.
> 
> Signed-off-by: Guanghui Yang <3497809730@qq.com>
> ---
> 
> Changes since v1:
> - Check dn.inode_folio before releasing ifolio to avoid a double put when
>    f2fs_reserve_block() has already cleared the dnode.
> 
>   fs/f2fs/data.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index a765fda71536..6c573d392dac 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1477,6 +1477,8 @@ struct folio *f2fs_get_new_data_folio(struct inode *inode,
>   	err = f2fs_reserve_block(&dn, index);
>   	if (err) {
>   		f2fs_folio_put(folio, true);
> +		if (dn.inode_folio)
> +			f2fs_folio_put(ifolio, true);

No, caller will handle it.

>   		return ERR_PTR(err);
>   	}
>   	if (!ifolio)
> 
> base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa

Re: [PATCH v2] f2fs: fix ifolio leak in f2fs_get_new_data_folio

[Guanghui Yang]

Resending as plain text because the mailing list rejected the previous
HTML reply.

Thanks for checking.

I agree, make_empty_dir() returns the error to f2fs_init_inode_metadata(),
and the caller handles the inode folio there.

Please ignore this patch.

Thanks,
Guanghui