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

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

On Tue, Jul 23, 2024 at 09:59:41AM -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 SECURE namespace attributes are not so very different,
> so allow them to use the reserved space as well.
> 
> Code-comment-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: Remove local variable, add comment.
> V3: Add Dave's preferred comment
> 
> diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
> index ab3d22f662f2..85e7be094943 100644
> --- a/fs/xfs/xfs_xattr.c
> +++ b/fs/xfs/xfs_xattr.c
> @@ -110,7 +110,26 @@ xfs_attr_change(
>  	args->whichfork = XFS_ATTR_FORK;
>  	xfs_attr_sethash(args);
>  
> -	return xfs_attr_set(args, op, args->attr_filter & XFS_ATTR_ROOT);
> +	/*
> +	 * Some xattrs must be resistent to allocation failure at

Nit: resistant

> +	 * 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.
> +	 */
> +	return xfs_attr_set(args, op,
> +			args->attr_filter & (XFS_ATTR_ROOT | XFS_ATTR_SECURE));

With that fixed,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  }
>  
>  
> 
> 
>