[PATCH] xfs_repair: always release btree buffers when refcountbt checks fail

[PATCH] xfs_repair: always release btree buffers when refcountbt checks fail

[Darrick J. Wong]

During phase 4 of repair, we compare the refcount records we've computed
against what's in the refcount btree.  If the btree is corrupt the
libxfs refcount calls will fail, but on our way out of the function we
don't make sure to clear all the btree cursor's buffer pointers.  This
leads to a repair deadlock in xfs/372 when fuzzing ptrs[1] to -1U
because we forgot to release a buffer.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/rmap.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/repair/rmap.c b/repair/rmap.c
index 01d6f0f..d51590b 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -1359,7 +1359,8 @@ _("Incorrect reference count: saw (%u/%u) len %u nlinks %u; should be (%u/%u) le
 
 err:
 	if (bt_cur)
-		libxfs_btree_del_cursor(bt_cur, XFS_BTREE_NOERROR);
+		libxfs_btree_del_cursor(bt_cur, error ? XFS_BTREE_ERROR :
+							XFS_BTREE_NOERROR);
 	if (agbp)
 		libxfs_putbuf(agbp);
 	free_slab_cursor(&rl_cur);

Re: [PATCH] xfs_repair: always release btree buffers when refcountbt checks fail

[Dave Chinner]

On Tue, Nov 07, 2017 at 04:26:36PM -0800, Darrick J. Wong wrote:
> During phase 4 of repair, we compare the refcount records we've computed
> against what's in the refcount btree.  If the btree is corrupt the
> libxfs refcount calls will fail, but on our way out of the function we
> don't make sure to clear all the btree cursor's buffer pointers.  This
> leads to a repair deadlock in xfs/372 when fuzzing ptrs[1] to -1U
> because we forgot to release a buffer.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  repair/rmap.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/repair/rmap.c b/repair/rmap.c
> index 01d6f0f..d51590b 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -1359,7 +1359,8 @@ _("Incorrect reference count: saw (%u/%u) len %u nlinks %u; should be (%u/%u) le
>  
>  err:
>  	if (bt_cur)
> -		libxfs_btree_del_cursor(bt_cur, XFS_BTREE_NOERROR);
> +		libxfs_btree_del_cursor(bt_cur, error ? XFS_BTREE_ERROR :
> +							XFS_BTREE_NOERROR);

Looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com