| Message ID | 7c666cfc-0478-42d0-b179-575ace474db0@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [V2] xfs: allow SECURE namespace xattrs to use reserved block pool | expand |
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.
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 + * (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. + */ + return xfs_attr_set(args, op, + args->attr_filter & (XFS_ATTR_ROOT | XFS_ATTR_SECURE)); }
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.