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

Chandan Babu R
2023-11-24T09:15:47Z

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