[PATCH 1/2] xfs: fix inode ref leak in attr intent recovery

[PATCH 1/2] xfs: fix inode ref leak in attr intent recovery

[Yingjie Gao]

xfs_attri_recover_work() grabs the target inode, attaches it to the
reconstructed attr work item, and adds that work item to the defer
pending list.

If xfs_attr_recover_work() fails to allocate the recovery transaction,
it returns immediately without dropping the inode reference.  The later
cancel path only frees the attr work item state, so the inode reference
leaks.

Release the inode before returning the transaction allocation failure.

Fixes: e70fb328d527 ("xfs: recreate work items when recovering intent items")
Cc: <stable@vger.kernel.org> # v6.8
Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
---
 fs/xfs/xfs_attr_item.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index deab14f31b38..c3d96c7a5bca 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -773,8 +773,10 @@ xfs_attr_recover_work(
 	}
 	resv = xlog_recover_resv(&resv);
 	error = xfs_trans_alloc(mp, &resv, total, 0, XFS_TRANS_RESERVE, &tp);
-	if (error)
+	if (error) {
+		xfs_irele(ip);
 		return error;
+	}
 	args->trans = tp;
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
-- 
2.20.1

Re: [PATCH 1/2] xfs: fix inode ref leak in attr intent recovery

[Darrick J. Wong]

On Tue, Jun 09, 2026 at 07:16:18PM +0800, Yingjie Gao wrote:
> xfs_attri_recover_work() grabs the target inode, attaches it to the
> reconstructed attr work item, and adds that work item to the defer
> pending list.
> 
> If xfs_attr_recover_work() fails to allocate the recovery transaction,
> it returns immediately without dropping the inode reference.  The later
> cancel path only frees the attr work item state, so the inode reference
> leaks.
> 
> Release the inode before returning the transaction allocation failure.
> 
> Fixes: e70fb328d527 ("xfs: recreate work items when recovering intent items")
> Cc: <stable@vger.kernel.org> # v6.8
> Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
> ---
>  fs/xfs/xfs_attr_item.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
> index deab14f31b38..c3d96c7a5bca 100644
> --- a/fs/xfs/xfs_attr_item.c
> +++ b/fs/xfs/xfs_attr_item.c
> @@ -773,8 +773,10 @@ xfs_attr_recover_work(
>  	}
>  	resv = xlog_recover_resv(&resv);
>  	error = xfs_trans_alloc(mp, &resv, total, 0, XFS_TRANS_RESERVE, &tp);
> -	if (error)
> +	if (error) {
> +		xfs_irele(ip);

Seems fine but I wonder why you don't just add an out_rele label on the
line above the existing xfs_irele() call and make this goto there?

--D

>  		return error;
> +	}
>  	args->trans = tp;
>  
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
> -- 
> 2.20.1
> 
> 

Re: [PATCH 1/2] xfs: fix inode ref leak in attr intent recovery

[Yingjie Gao]

在 2026/6/9 22:57, Darrick J. Wong 写道:
> On Tue, Jun 09, 2026 at 07:16:18PM +0800, Yingjie Gao wrote:
>> xfs_attri_recover_work() grabs the target inode, attaches it to the
>> reconstructed attr work item, and adds that work item to the defer
>> pending list.
>>
>> If xfs_attr_recover_work() fails to allocate the recovery transaction,
>> it returns immediately without dropping the inode reference.  The later
>> cancel path only frees the attr work item state, so the inode reference
>> leaks.
>>
>> Release the inode before returning the transaction allocation failure.
>>
>> Fixes: e70fb328d527 ("xfs: recreate work items when recovering intent items")
>> Cc: <stable@vger.kernel.org> # v6.8
>> Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
>> ---
>>  fs/xfs/xfs_attr_item.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
>> index deab14f31b38..c3d96c7a5bca 100644
>> --- a/fs/xfs/xfs_attr_item.c
>> +++ b/fs/xfs/xfs_attr_item.c
>> @@ -773,8 +773,10 @@ xfs_attr_recover_work(
>>  	}
>>  	resv = xlog_recover_resv(&resv);
>>  	error = xfs_trans_alloc(mp, &resv, total, 0, XFS_TRANS_RESERVE, &tp);
>> -	if (error)
>> +	if (error) {
>> +		xfs_irele(ip);
> 
> Seems fine but I wonder why you don't just add an out_rele label on the
> line above the existing xfs_irele() call and make this goto there?
> 
> --D
> 
Good point, I'll update the patch and send a v2.
Thanks for the review.

-- 
Yingjie
>>  		return error;
>> +	}
>>  	args->trans = tp;
>>  
>>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>> -- 
>> 2.20.1
>>
>>
> 

[PATCH v2 1/2] xfs: fix inode ref leak in attr intent recovery

[Yingjie Gao]

xfs_attri_recover_work() grabs the target inode, attaches it to the
reconstructed attr work item, and adds that work item to the defer
pending list.

If xfs_attr_recover_work() fails to allocate the recovery transaction,
it returns immediately without dropping the inode reference.  The later
cancel path only frees the attr work item state, so the inode reference
leaks.

Send the failure through the existing cleanup path so the inode
reference is dropped before the function returns the error.

Fixes: e70fb328d527 ("xfs: recreate work items when recovering intent items")
Cc: <stable@vger.kernel.org> # v6.8
Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
---
 fs/xfs/xfs_attr_item.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index deab14f31b38..841838bc1d0f 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -774,7 +774,7 @@ xfs_attr_recover_work(
 	resv = xlog_recover_resv(&resv);
 	error = xfs_trans_alloc(mp, &resv, total, 0, XFS_TRANS_RESERVE, &tp);
 	if (error)
-		return error;
+		goto out_rele;
 	args->trans = tp;
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
@@ -791,6 +791,7 @@ xfs_attr_recover_work(
 	error = xfs_defer_ops_capture_and_commit(tp, capture_list);
 out_unlock:
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+out_rele:
 	xfs_irele(ip);
 	return error;
 out_cancel:
-- 
2.20.1

Re: [PATCH v2 1/2] xfs: fix inode ref leak in attr intent recovery

[Darrick J. Wong]

On Wed, Jun 10, 2026 at 10:20:27AM +0800, Yingjie Gao wrote:
> xfs_attri_recover_work() grabs the target inode, attaches it to the
> reconstructed attr work item, and adds that work item to the defer
> pending list.
> 
> If xfs_attr_recover_work() fails to allocate the recovery transaction,
> it returns immediately without dropping the inode reference.  The later
> cancel path only frees the attr work item state, so the inode reference
> leaks.
> 
> Send the failure through the existing cleanup path so the inode
> reference is dropped before the function returns the error.
> 
> Fixes: e70fb328d527 ("xfs: recreate work items when recovering intent items")
> Cc: <stable@vger.kernel.org> # v6.8
> Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>

Looks good,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_attr_item.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
> index deab14f31b38..841838bc1d0f 100644
> --- a/fs/xfs/xfs_attr_item.c
> +++ b/fs/xfs/xfs_attr_item.c
> @@ -774,7 +774,7 @@ xfs_attr_recover_work(
>  	resv = xlog_recover_resv(&resv);
>  	error = xfs_trans_alloc(mp, &resv, total, 0, XFS_TRANS_RESERVE, &tp);
>  	if (error)
> -		return error;
> +		goto out_rele;
>  	args->trans = tp;
>  
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
> @@ -791,6 +791,7 @@ xfs_attr_recover_work(
>  	error = xfs_defer_ops_capture_and_commit(tp, capture_list);
>  out_unlock:
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +out_rele:
>  	xfs_irele(ip);
>  	return error;
>  out_cancel:
> -- 
> 2.20.1
> 
>