| Message ID | 20210826211937.317558-1-vgoyal@redhat.com |
|---|---|
| Headers | show |
| Series | virtiofsd: Add capability to block xattrs | expand |
* Vivek Goyal (vgoyal@redhat.com) wrote: > As of now we have a knob "-o xattr/no_xattr" which either enables > all xattrs or disables all xattrs. Hi Vivek, Thanks for this. > We need something more fine grained where we can selectively disable > only certain xattrs (and not all). > > For example, in some cases we want to disable "security.selinux" > xattr. This is equivalent to virtiofs not supporting security.selinux > and guest kernel will fallback to a single label for whole fs > (virtiofs_t). > > So add an option "-o block_xattr=<list-of-xattrs>" which will allow > specifying a list of xattrs to block. This is quite interesting; I'd not noticed you had the exisitng blocking mechanism, however, as discussed, I think my preference is if this could be done as a modification of the xattrmap it would avoid another set of options. The mapping code already has 'type's of: prefix, ok, bad I think you just need to add a 'reject' type that produces the error code you need. Dave > Vivek Goyal (2): > virtiofsd: Add an array to keep track of blocked xattrs > virtiofsd: Add option "block_xattr=" to block certain xattrs > > docs/tools/virtiofsd.rst | 17 ++++ > tools/virtiofsd/helper.c | 3 + > tools/virtiofsd/passthrough_ll.c | 166 ++++++++++++++++++++++++++++--- > 3 files changed, 171 insertions(+), 15 deletions(-) > > -- > 2.31.1 >
On Wed, Sep 22, 2021 at 12:00:17PM +0100, Dr. David Alan Gilbert wrote: > * Vivek Goyal (vgoyal@redhat.com) wrote: > > As of now we have a knob "-o xattr/no_xattr" which either enables > > all xattrs or disables all xattrs. > > Hi Vivek, > Thanks for this. > > > We need something more fine grained where we can selectively disable > > only certain xattrs (and not all). > > > > For example, in some cases we want to disable "security.selinux" > > xattr. This is equivalent to virtiofs not supporting security.selinux > > and guest kernel will fallback to a single label for whole fs > > (virtiofs_t). > > > > So add an option "-o block_xattr=<list-of-xattrs>" which will allow > > specifying a list of xattrs to block. > > This is quite interesting; I'd not noticed you had the exisitng blocking > mechanism, Yes, that's for blocking posix acl xattrs if needed. If xattr map support blocking, then we could probably insert an internal rule to block posix acl xattrs. But that's more of a cleanup exercise I will take up some other time. > however, as discussed, I think my preference is if this could > be done as a modification of the xattrmap it would avoid another set of > options. > > The mapping code already has 'type's of: > > prefix, ok, bad > > I think you just need to add a 'reject' type > that produces the error code you need. How about "unsupported" and then return -EOPNOTSUPP? I am looking at selinux kernel code and it expect -EOPNOTSUPP to decide that selinux xattr is not supported and looks into fallback options. static int sb_check_xattr_support(struct super_block *sb) { rc = __vfs_getxattr(root, root_inode, XATTR_NAME_SELINUX, NULL, 0); if (rc < 0 && rc != -ENODATA) { if (rc == -EOPNOTSUPP) { pr_warn("SELinux: (dev %s, type %s) has no security xattr handler\n", sb->s_id, sb->s_type->name); goto fallback; ... ... } Vivek