On Tue, Mar 16, 2021 at 02:21:45PM -0400, Paul Moore wrote: > On Tue, Mar 16, 2021 at 10:48 AM Ondrej Mosnacek <omosn...@redhat.com> wrote: > > > > When SELinux security options are passed to btrfs via fsconfig(2) rather > > than via mount(2), the operation aborts with an error. What happens is > > roughly this sequence: > > > > 1. vfs_parse_fs_param() eats away the LSM options and parses them into > > fc->security. > > 2. legacy_get_tree() finds nothing in ctx->legacy_data, passes this > > nothing to btrfs. > > [here btrfs calls another layer of vfs_kern_mount(), but let's ignore > > that for simplicity]
Let's not. This is where the root of the problem actually lies. Take a look at that sucker: struct fs_context *fc; struct vfsmount *mnt; int ret = 0; if (!type) return ERR_PTR(-EINVAL); fc = fs_context_for_mount(type, flags); if (IS_ERR(fc)) return ERR_CAST(fc); if (name) ret = vfs_parse_fs_string(fc, "source", name, strlen(name)); if (!ret) ret = parse_monolithic_mount_data(fc, data); if (!ret) mnt = fc_mount(fc); else mnt = ERR_PTR(ret); put_fs_context(fc); return mnt; That's where the problem comes - you've lost the original context's ->security. Note that there's such thing as security_fs_context_dup(), so you can bloody well either * provide a variant of vfs_kern_mount() that would take 'base' fc to pick security options from or * do all options parsing on btrfs fc and then do fs_context_for_mount + security_fs_context_dup + copy (parsed) options to whatever private data you use for btrfs_root context + fc_mount + put_fs_context yourself. My preference would be the latter, but I have *not* looked at btrfs mount options handling in any details. > VFS folks, can we get a verdict/feedback on this patch? The v1 draft > of this patch was posted almost four months ago with no serious > comments/feedback. It's a bit ugly, but it does appear to work and at > the very least SELinux needs this to handle btrfs properly, other LSMs > may need this too. It's more than a bit ugly; it perpetuates the use of FS_BINARY_MOUNTDATA, and the semantics it gets is quite counterintuitive at that.