[PATCH] xfs: fix temporary cursor leak in btree shift corruption paths

[PATCH] xfs: fix temporary cursor leak in btree shift corruption paths

[jianyungao89]

From: Jianyun Gao <jianyungao89@gmail.com>

In xfs_btree_lshift and xfs_btree_rshift, when XFS_IS_CORRUPT detects
an invalid firstrec/lastrec result after xfs_btree_dup_cursor has
successfully allocated a temporary cursor, the error path jumps to
error0 which returns without freeing tcur.  This leaks the cursor
memory, the xfs_group reference, and the per-level buffer references
held by the duplicated cursor.  Jump to error1 instead, which calls
xfs_btree_del_cursor(tcur, XFS_BTREE_ERROR) to properly release all
resources.

Cc: stable@vger.kernel.org
Fixes: 9eaead51bed95 ("[XFS] implement generic xfs_btree_rshift")
Signed-off-by: Jianyun Gao <jianyungao89@gmail.com>
---
 fs/xfs/libxfs/xfs_btree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 7effebe297ea..d706f59b0301 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -2567,7 +2567,7 @@ xfs_btree_lshift(
 		if (XFS_IS_CORRUPT(tcur->bc_mp, i != 1)) {
 			xfs_btree_mark_sick(cur);
 			error = -EFSCORRUPTED;
-			goto error0;
+			goto error1;
 		}
 
 		error = xfs_btree_decrement(tcur, level, &i);
@@ -2737,7 +2737,7 @@ xfs_btree_rshift(
 	if (XFS_IS_CORRUPT(tcur->bc_mp, i != 1)) {
 		xfs_btree_mark_sick(cur);
 		error = -EFSCORRUPTED;
-		goto error0;
+		goto error1;
 	}
 
 	error = xfs_btree_increment(tcur, level, &i);
-- 
2.34.1

Re: [PATCH] xfs: fix temporary cursor leak in btree shift corruption paths

[Carlos Maiolino]

On Mon, Nov 02, 2026 at 11:01:41PM +0800, jianyungao89@gmail.com wrote:
> From: Jianyun Gao <jianyungao89@gmail.com>
> 
> In xfs_btree_lshift and xfs_btree_rshift, when XFS_IS_CORRUPT detects
> an invalid firstrec/lastrec result after xfs_btree_dup_cursor has
> successfully allocated a temporary cursor, the error path jumps to
> error0 which returns without freeing tcur.  This leaks the cursor
> memory, the xfs_group reference, and the per-level buffer references
> held by the duplicated cursor.  Jump to error1 instead, which calls
> xfs_btree_del_cursor(tcur, XFS_BTREE_ERROR) to properly release all
> resources.
> 
> Cc: stable@vger.kernel.org
> Fixes: 9eaead51bed95 ("[XFS] implement generic xfs_btree_rshift")
> Signed-off-by: Jianyun Gao <jianyungao89@gmail.com>

Yup, that's leaking the duplicated cursor

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_btree.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 7effebe297ea..d706f59b0301 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -2567,7 +2567,7 @@ xfs_btree_lshift(
>  		if (XFS_IS_CORRUPT(tcur->bc_mp, i != 1)) {
>  			xfs_btree_mark_sick(cur);
>  			error = -EFSCORRUPTED;
> -			goto error0;
> +			goto error1;
>  		}
>  
>  		error = xfs_btree_decrement(tcur, level, &i);
> @@ -2737,7 +2737,7 @@ xfs_btree_rshift(
>  	if (XFS_IS_CORRUPT(tcur->bc_mp, i != 1)) {
>  		xfs_btree_mark_sick(cur);
>  		error = -EFSCORRUPTED;
> -		goto error0;
> +		goto error1;
>  	}
>  
>  	error = xfs_btree_increment(tcur, level, &i);
> -- 
> 2.34.1
> 

Re: [PATCH] xfs: fix temporary cursor leak in btree shift corruption paths

[Carlos Maiolino]

On Mon, 02 Nov 2026 23:01:41 +0800, jianyungao89@gmail.com wrote:
> In xfs_btree_lshift and xfs_btree_rshift, when XFS_IS_CORRUPT detects
> an invalid firstrec/lastrec result after xfs_btree_dup_cursor has
> successfully allocated a temporary cursor, the error path jumps to
> error0 which returns without freeing tcur.  This leaks the cursor
> memory, the xfs_group reference, and the per-level buffer references
> held by the duplicated cursor.  Jump to error1 instead, which calls
> xfs_btree_del_cursor(tcur, XFS_BTREE_ERROR) to properly release all
> resources.
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: fix temporary cursor leak in btree shift corruption paths
      commit: 2bbca049deeb1699cafd966cb2793fee576ee409

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>

Re: [PATCH] xfs: fix temporary cursor leak in btree shift corruption paths

[Markus Elfring]

…
> held by the duplicated cursor.  Jump to error1 instead, which calls
> xfs_btree_del_cursor(tcur, XFS_BTREE_ERROR) to properly release all
> resources.

Can further source code clean-up become helpful for affected function implementations?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v7.4-rc1#n526
	
Regards,
Markus

Re: [PATCH] xfs: fix temporary cursor leak in btree shift corruption paths

[Jianyun Gao]

Hi Markus,
Thanks for the review and the suggestion. I agree that centralizing
the error handling paths in these functions would improve readability.
However, to keep this patch focused strictly on the cursor leak fix
(as per the 'one patch, one thing' principle), I would prefer to
submit the code clean-up as a separate follow-up patch if that's
acceptable. I will review the affected functions and see if there are
any obvious style issues worth addressing.
Best regards,
Jianyun

On Tue, Nov 3, 2026 at 6:43 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> …
> > held by the duplicated cursor.  Jump to error1 instead, which calls
> > xfs_btree_del_cursor(tcur, XFS_BTREE_ERROR) to properly release all
> > resources.
>
> Can further source code clean-up become helpful for affected function implementations?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v7.4-rc1#n526
>
> Regards,
> Markus

Re: xfs: fix temporary cursor leak in btree shift corruption paths

[Markus Elfring]

> Thanks for the review and the suggestion. I agree that centralizing
> the error handling paths in these functions would improve readability.

Further adjustment possibilities might be evolving.


> However, to keep this patch focused strictly on the cursor leak fix
> (as per the 'one patch, one thing' principle), I would prefer to
> submit the code clean-up as a separate follow-up patch if that's
> acceptable.

This can be the case.
Related constraints can eventually be reconsidered also for clean-up patches
in combination with growing number for fixes.


>             I will review the affected functions and see if there are
> any obvious style issues worth addressing.

Would you find another clarification approach helpful?

[RFC] Replacing special goto statements by direct returns?
https://lore.kernel.org/kernel-janitors/52418f50-b3ff-461d-8e66-c10f70e35b2a@web.de/

Regards,
Markus

Re: [PATCH] xfs: fix temporary cursor leak in btree shift corruption paths

[Christoph Hellwig]

Looks good:

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