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

["Darrick J. Wong" <djwong@kernel.org>]

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
> 
>