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

Darrick J. Wong
2024-04-03T03:53:15Z

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