Re: [PATCH 3/4] xfs: handle allocation failure in xfs_dquot_disk_alloc()

Darrick J. Wong
2024-04-03T03:48:56Z

On Wed, Apr 03, 2024 at 08:38:18AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> If free space accounting is screwed up, then dquot allocation may go
> ahead when there is no space available. xfs_dquot_disk_alloc() does
> not handle allocation failure - it expects that it will not get
> called when there isn't space available to allocate dquots.
>
> Because fuzzers have been screwing up the free space accounting, we
> are seeing failures in dquot allocation, and they aren't being
> caught on produciton kernels. Debug kernels will assert fail in this
> case, so turn that assert fail into more robust error handling to
> avoid these issues in future.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Sounds fine to me! It'll be interesting to see what happens the next
time one of my VMs trips this.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> fs/xfs/xfs_dquot.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index c98cb468c357..a2652e3d5164 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -356,6 +356,23 @@ xfs_dquot_disk_alloc(
> if (error)
> goto err_cancel;
>
> + if (nmaps == 0) {
> + /*
> + * Unexpected ENOSPC - the transaction reservation should have
> + * guaranteed that this allocation will succeed. We don't know
> + * why this happened, so just back out gracefully.
> + *
> + * We commit the transaction instead of cancelling it as it may
> + * be dirty due to extent count upgrade. This avoids a potential
> + * filesystem shutdown when this happens. We ignore any error
> + * from the transaction commit - we always return -ENOSPC to the
> + * caller here so we really don't care if the commit fails for
> + * some unknown reason...
> + */
> + xfs_trans_commit(tp);
> + return -ENOSPC;
> + }
> +
> ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
> ASSERT(nmaps == 1);
> ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
> --
> 2.43.0
>
>