On Fri, 2021-12-03 at 13:06 -0500, Stefan Berger wrote:
On 12/3/21 12:03, James Bottomley wrote:Well securityfs is supposed to exist for LSMs. At some point each of
On Thu, 2021-12-02 at 21:31 -0500, Stefan Berger wrote:I had thought about this also but the rationale was:
[...]
static int securityfs_init_fs_context(struct fs_context *fc)I know I suggested this, but to get this to work in general, it's
{
+ int rc;
+
+ if (fc->user_ns->ima_ns->late_fs_init) {
+ rc = fc->user_ns->ima_ns->late_fs_init(fc->user_ns);
+ if (rc)
+ return rc;
+ }
fc->ops = &securityfs_context_ops;
return 0;
}
going to have to not be specific to IMA, so it's going to have to
become something generic like a notifier chain. The other problem
is it's only working still by accident:
securityfs is compiled due to CONFIG_IMA_NS and the user namespace
exists there and that has a pointer now to ima_namespace, which can
have that callback. I assumed that other namespaced subsystems could
also be reached then via such a callback, but I don't know.
those is going to need to be namespaced, which may eventually be quite
a pile of callbacks, which is why I thought of a notifier.
I suppose any late filesystem init callchain would have to beI don't think so; I think just moving some securityfs entries into the
connected to the user_namespace somehow?
user_namespace and managing the notifier chain from within securityfs
will do for now. [although I'd have to spec this out in code before I
knew for sure].
Right, it's being activated by securityfs_ns_create_mount which isWhat I don't like about the fill_super is that it gets called too+int ima_fs_ns_init(struct ima_namespace *ns)This actually triggers on the call to securityfs_init_fs_context,
+{
+ ns->mount = securityfs_ns_create_mount(ns->user_ns);
but nothing happens because the callback is null. Every subsequent
use of fscontext will trigger this. The point of a keyed supeblock
is that fill_super is only called once per key, that's the place we
should be doing this. It should also probably be a blocking
notifier so anyconsumer of securityfs can be namespaced by
registering for this notifier.
early:
[ 67.058611] securityfs_ns_create_mount @ 102 target user_ns:
ffff95c010698c80; nr_extents: 0
[ 67.059836] securityfs_fill_super @ 47 user_ns:
ffff95c010698c80;
nr_extents: 0
called as soon as the user_ns is created.
We are switching to the target user namespace inExactly, so I was thinking of not having a securityfs_ns_create_mount
securityfs_ns_create_mount. The expected nr_extents at this point is
0, since user_ns hasn't been configured, yet. But then
security_fill_super is also called with nr_extents 0. We cannot use
that, it's too early!
at all. All the securityfs_ns_create.. calls would be in the notifier
call chain. This means there's nothing to fill the superblock until an
actual mount on it is called.
So it would be called early in the init_user_ns by non-namespacedOk, I can make this disappear.+ if (IS_ERR(ns->mount)) {This is a bit nasty, too: we're spilling the guts of mount count
+ ns->mount = NULL;
+ return -1;
+ }
+ ns->mount_count = 1;
tracking into IMA instead of encapsulating it inside securityfs.
fill_super would only work for the init_user_ns from what I can see.+I think what should be happening is that we shouldn't so the
+ /* Adjust the trigger for user namespace's early teardown of
dependent
+ * namespaces. Due to the filesystem there's an additional
reference
+ * to the user namespace.
+ */
+ ns->user_ns->refcount_teardown += 1;
+
+ ns->late_fs_init = ima_fs_ns_late_init;
+
+ return 0;
+}
simple_pin_fs, which creates the inodes, ahead of time; we should
do it inside fill_super using a notifier, meaning it gets called
once per
key, creates the root dentry then triggers the notifier whichI guess I would need to know how fill_super would work or how it
instantiates all the namespaced entries. We can still use
simple_pin_fs for this because there's no locking across
fill_super.
This would mean fill_super would be called the first time the
securityfs is mounted inside the namespace.
could be called late/delayed as well.
consumers of securityfs, like it is now.
Namespaced consumers wouldn't call any securityfs_ns_create callbacks
to create dentries until they were notified from the fill_super
notifier, which would now only be triggered on first mount of
securityfs inside the namespace.
I don't think so no. It could be entirely managed internally toIf we do it this way, we can now make securityfs have its own mountIs there then no need to pass a separate vfsmount * in anymore?
and mount_count inside the user namespace, which it uses internally
to the securityfs code, thus avoiding exposing them to ima or any
other namespaced consumer.
I also think we now don't need the securityfs_ns_ duplicated
functions because the callback via the notifier chain now ensures
we can usethe namespace they were created in to distinguish between
non namespaced and namespaced entries.
securityfs.
Where would the vfsmount pointer reside? For now it's inexactly. I think struct user_namespace should have two elements gated
ima_namespace, but it sounds like it should be in a more centralized
place? Should it also be connected to the user_namespace so we can
pick it up using get_user_ns()?
by a #ifdef CONFIG_SECURITYFS which are the vfsmount and the
mount_count for passing into simple_pin_fs.
James