[PATCH 1/2] xfs: fix ilock leak on error in xfs_dq_get_next_id

[PATCH 1/2] xfs: fix ilock leak on error in xfs_dq_get_next_id

[Long Li]

xfs_dq_get_next_id() takes the quota inode ILOCK before calling
xfs_iread_extents().  If xfs_iread_extents() fails, the function returns
immediately without releasing the lock, leaking the quota inode ILOCK.
This can leave the quota inode locked and cause subsequent quota
operations to hang.

Fix this by jumping to a common unlock path on error instead of returning
directly.

Fixes: bda250dbaf39f ("xfs: rewrite xfs_dq_get_next_id using xfs_iext_lookup_extent")
Cc: <stable@vger.kernel.org> # v4.12
Signed-off-by: Long Li <leo.lilong@huawei.com>
---
 fs/xfs/xfs_dquot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index c311f61d9554..b4f6c594808c 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -778,7 +778,7 @@ xfs_dq_get_next_id(
 	lock_flags = xfs_ilock_data_map_shared(quotip);
 	error = xfs_iread_extents(NULL, quotip, XFS_DATA_FORK);
 	if (error)
-		return error;
+		goto out_unlock;
 
 	if (xfs_iext_lookup_extent(quotip, &quotip->i_df, start, &cur, &got)) {
 		/* contiguous chunk, bump startoff for the id calculation */
@@ -789,6 +789,7 @@ xfs_dq_get_next_id(
 		error = -ENOENT;
 	}
 
+out_unlock:
 	xfs_iunlock(quotip, lock_flags);
 
 	return error;
-- 
2.52.0

[PATCH 2/2] xfs: don't swallow dquot recovery verification errors

[Long Li]

xlog_recover_dquot_commit_pass2() validates the recovered dquot with
xfs_dqblk_verify() and, on failure, sets error = -EFSCORRUPTED and jumps
to out_release.  But out_release unconditionally returns 0, so the
corruption error is discarded: the caller xlog_recover_items_pass2()
sees success, log recovery proceeds as if the dquot were valid, and the
corrupt quota buffer can be written back to disk.

Fixes: 9c235dfc3d3f ("xfs: dquot recovery does not validate the recovered dquot")
Cc: <stable@vger.kernel.org> # v6.8
Signed-off-by: Long Li <leo.lilong@huawei.com>
---
 fs/xfs/xfs_dquot_item_recover.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c
index fe419b28de22..63bc9ab7d947 100644
--- a/fs/xfs/xfs_dquot_item_recover.c
+++ b/fs/xfs/xfs_dquot_item_recover.c
@@ -173,7 +173,7 @@ xlog_recover_dquot_commit_pass2(
 
 out_release:
 	xfs_buf_relse(bp);
-	return 0;
+	return error;
 }
 
 const struct xlog_recover_item_ops xlog_dquot_item_ops = {
-- 
2.52.0

Re: [PATCH 2/2] xfs: don't swallow dquot recovery verification errors

[Darrick J. Wong]

On Mon, Jul 27, 2026 at 10:38:49AM +0800, Long Li wrote:
> xlog_recover_dquot_commit_pass2() validates the recovered dquot with
> xfs_dqblk_verify() and, on failure, sets error = -EFSCORRUPTED and jumps
> to out_release.  But out_release unconditionally returns 0, so the
> corruption error is discarded: the caller xlog_recover_items_pass2()
> sees success, log recovery proceeds as if the dquot were valid, and the
> corrupt quota buffer can be written back to disk.
> 
> Fixes: 9c235dfc3d3f ("xfs: dquot recovery does not validate the recovered dquot")
> Cc: <stable@vger.kernel.org> # v6.8
> Signed-off-by: Long Li <leo.lilong@huawei.com>

Yep, that also looks like a bug
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_dquot_item_recover.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c
> index fe419b28de22..63bc9ab7d947 100644
> --- a/fs/xfs/xfs_dquot_item_recover.c
> +++ b/fs/xfs/xfs_dquot_item_recover.c
> @@ -173,7 +173,7 @@ xlog_recover_dquot_commit_pass2(
>  
>  out_release:
>  	xfs_buf_relse(bp);
> -	return 0;
> +	return error;
>  }
>  
>  const struct xlog_recover_item_ops xlog_dquot_item_ops = {
> -- 
> 2.52.0
> 
> 

Re: [PATCH 2/2] xfs: don't swallow dquot recovery verification errors

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] xfs: fix ilock leak on error in xfs_dq_get_next_id

[Darrick J. Wong]

On Mon, Jul 27, 2026 at 10:38:48AM +0800, Long Li wrote:
> xfs_dq_get_next_id() takes the quota inode ILOCK before calling
> xfs_iread_extents().  If xfs_iread_extents() fails, the function returns
> immediately without releasing the lock, leaking the quota inode ILOCK.
> This can leave the quota inode locked and cause subsequent quota
> operations to hang.
> 
> Fix this by jumping to a common unlock path on error instead of returning
> directly.
> 
> Fixes: bda250dbaf39f ("xfs: rewrite xfs_dq_get_next_id using xfs_iext_lookup_extent")
> Cc: <stable@vger.kernel.org> # v4.12
> Signed-off-by: Long Li <leo.lilong@huawei.com>

Yeah, that's a bug!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_dquot.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index c311f61d9554..b4f6c594808c 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -778,7 +778,7 @@ xfs_dq_get_next_id(
>  	lock_flags = xfs_ilock_data_map_shared(quotip);
>  	error = xfs_iread_extents(NULL, quotip, XFS_DATA_FORK);
>  	if (error)
> -		return error;
> +		goto out_unlock;
>  
>  	if (xfs_iext_lookup_extent(quotip, &quotip->i_df, start, &cur, &got)) {
>  		/* contiguous chunk, bump startoff for the id calculation */
> @@ -789,6 +789,7 @@ xfs_dq_get_next_id(
>  		error = -ENOENT;
>  	}
>  
> +out_unlock:
>  	xfs_iunlock(quotip, lock_flags);
>  
>  	return error;
> -- 
> 2.52.0
> 
> 

Re: [PATCH 1/2] xfs: fix ilock leak on error in xfs_dq_get_next_id

[Christoph Hellwig]

On Mon, Jul 27, 2026 at 10:38:48AM +0800, Long Li wrote:
> xfs_dq_get_next_id() takes the quota inode ILOCK before calling
> xfs_iread_extents().  If xfs_iread_extents() fails, the function returns
> immediately without releasing the lock, leaking the quota inode ILOCK.
> This can leave the quota inode locked and cause subsequent quota
> operations to hang.
> 
> Fix this by jumping to a common unlock path on error instead of returning
> directly.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>