Re: [PATCH 2/9] libxfs: don't UAF a requeued EFI

[Chandan Babu R <chandanbabu@kernel.org>]

On Wed, Nov 22, 2023 at 03:06:59 PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> In the kernel, commit 8ebbf262d4684 ("xfs: don't block in busy flushing
> when freeing extents") changed the allocator behavior such that AGFL
> fixing can return -EAGAIN in response to detection of a deadlock with
> the transaction busy extent list.  If this happens, we're supposed to
> requeue the EFI so that we can roll the transaction and try the item
> again.
>
> If a requeue happens, we should not free the xefi pointer in
> xfs_extent_free_finish_item or else the retry will walk off a dangling
> pointer.  There is no extent busy list in userspace so this should
> never happen, but let's fix the logic bomb anyway.
>
> We should have ported kernel commit 0853b5de42b47 ("xfs: allow extent
> free intents to be retried") to userspace, but neither Carlos nor I
> noticed this fine detail. :(
>

Looks good to me.

Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>

> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  libxfs/defer_item.c |    7 +++++++
>  1 file changed, 7 insertions(+)
>
>
> diff --git a/libxfs/defer_item.c b/libxfs/defer_item.c
> index 3f519252046..8731d1834be 100644
> --- a/libxfs/defer_item.c
> +++ b/libxfs/defer_item.c
> @@ -115,6 +115,13 @@ xfs_extent_free_finish_item(
>  	error = xfs_free_extent(tp, xefi->xefi_pag, agbno,
>  			xefi->xefi_blockcount, &oinfo, XFS_AG_RESV_NONE);
>  
> +	/*
> +	 * Don't free the XEFI if we need a new transaction to complete
> +	 * processing of it.
> +	 */
> +	if (error == -EAGAIN)
> +		return error;
> +
>  	xfs_extent_free_put_group(xefi);
>  	kmem_cache_free(xfs_extfree_item_cache, xefi);
>  	return error;


-- 
Chandan