Re: [PATCH V2] xfs: allow SECURE namespace xattrs to use reserved block pool

[Dave Chinner <david@fromorbit.com>]

On Mon, Jul 22, 2024 at 02:25:33PM -0500, Eric Sandeen wrote:
> We got a report from the podman folks that selinux relabels that happen
> as part of their process were returning ENOSPC when the filesystem is
> completely full. This is because xattr changes reserve about 15 blocks
> for the worst case, but the common case is for selinux contexts to be
> the sole, in-inode xattr and consume no blocks.
> 
> We already allow reserved space consumption for XFS_ATTR_ROOT for things
> such as ACLs, and selinux / SECURE attributes are not so very different,
> so allow them to use the reserved space as well.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: Remove local variable, add comment.
> 
> diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
> index ab3d22f662f2..09f004af7672 100644
> --- a/fs/xfs/xfs_xattr.c
> +++ b/fs/xfs/xfs_xattr.c
> @@ -110,7 +110,16 @@ xfs_attr_change(
>  	args->whichfork = XFS_ATTR_FORK;
>  	xfs_attr_sethash(args);
>  
> -	return xfs_attr_set(args, op, args->attr_filter & XFS_ATTR_ROOT);
> +	/*
> +	 * Allow xattrs for ACLs (ROOT namespace) and SELinux contexts

It's not just SELinux - it's security xattrs set by LSMs in general
that use the SECURE namespace. These come through:

xfs_generic_create()
  xfs_inode_init_security()
    security_inode_init_security()
      <LSM>
        xfs_initxattrs()
          xfs_attr_change(XFS_ATTR_SECURE)

> +	 * (SECURE namespace) to use the reserved block pool for these
> +	 * security-related operations. xattrs typically reside in the inode,
> +	 * so in many cases the reserved pool won't actually get consumed,
> +	 * but this will help the worst-case transaction reservations to
> +	 * succeed.
> +	 */

It doesn't explain why we need this - it's got the what and the
expected behaviour, but no why. :)

> +	return xfs_attr_set(args, op,
> +		    args->attr_filter & (XFS_ATTR_ROOT | XFS_ATTR_SECURE));
>  }

Perhaps it would be better to say something like:

	/*
	 * Some xattrs must be resistent to allocation failure at
	 * ENOSPC. e.g. creating an inode with ACLs or security
	 * attributes requires the allocation of the xattr holding
	 * that information to succeed. Hence we allow xattrs in the
	 * VFS TRUSTED, SYSTEM, POSIX_ACL and SECURITY (LSM xattr)
	 * namespaces to dip into the reserve block pool to allow
	 * manipulation of these xattrs when at ENOSPC. These VFS
	 * xattr namespaces translate to the XFS_ATTR_ROOT and
	 * XFS_ATTR_SECURE on-disk namespaces.
	 *
	 * For most of these cases, these special xattrs will fit in
	 * the inode itself and so consume no extra space or only
	 * require temporary extra space while an overwrite is being
	 * made. Hence the use of the reserved pool is largely to
	 * avoid the worst case reservation from preventing the
	 * xattr from being created at ENOSPC.
	 */

-Dave.
-- 
Dave Chinner
david@fromorbit.com