Re: [PATCH 4/4] xfs: validate block count for XFS_IOC_SET_RESBLKS

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

On Wed, Apr 03, 2024 at 08:38:19AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Userspace can pass anything it wants in the reserved block count
> and we simply pass that to the reservation code. If a value that is
> far too large is passed, we can overflow the free space counter
> and df reports things like:
> 
> Filesystem      Size  Used Avail Use% Mounted on
> /dev/loop0       14M  -27Z   27Z    - /home/dave/bugs/file0
> 
> As reserving space requires CAP_SYS_ADMIN, this is not a problem
> that will ever been seen in production systems. However, fuzzers are
> running with CAP_SYS_ADMIN, and so they able to run filesystem code
> with out-of-band free space accounting.
> 
> Stop the fuzzers ifrom being able to do this by validating that the
> count is within the bounds of the filesystem size and reject
> anything outside those bounds as invalid.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_ioctl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index d0e2cec6210d..18a225d884dd 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1892,6 +1892,9 @@ xfs_ioctl_getset_resblocks(
>  		if (copy_from_user(&fsop, arg, sizeof(fsop)))
>  			return -EFAULT;
>  
> +		if (fsop.resblks >= mp->m_sb.sb_dblocks)
> +			return -EINVAL;

Why isn't xfs_reserve_blocks catching this?  Is this due to the odd
behavior that a failed xfs_mod_fdblocks is undone and m_resblks simply
allowed to remain?

Also why wouldn't we limit m_resblks to something smaller, like 10% of
the fs or half an AG or something like that?

--D

> +
>  		error = mnt_want_write_file(filp);
>  		if (error)
>  			return error;
> -- 
> 2.43.0
> 
>