On 12/3/21 13:50, James Bottomley wrote:
On Fri, 2021-12-03 at 13:06 -0500, Stefan Berger wrote:
On 12/3/21 12:03, James Bottomley wrote:
On Thu, 2021-12-02 at 21:31 -0500, Stefan Berger wrote:
[...]
static int securityfs_init_fs_context(struct fs_context *fc)
{
+ 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;
}
I know I suggested this, but to get this to work in general, it's
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:
I had thought about this also but the rationale was:
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.
Well securityfs is supposed to exist for LSMs. At some point each of
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 be
connected to the user_namespace somehow?
I don't think so; I think just moving some securityfs entries into the
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].
It doesn't have to be right in the user_namespace. The IMA namespace is
connected to the user namespace and holds the dentries now...
Please spec it out...
+int ima_fs_ns_init(struct ima_namespace *ns)
+{
+ ns->mount = securityfs_ns_create_mount(ns->user_ns);
This actually triggers on the call to securityfs_init_fs_context,
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.
What I don't like about the fill_super is that it gets called too
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
Right, it's being activated by securityfs_ns_create_mount which is
called as soon as the user_ns is created.
Well, that doesn't help us then...
We are switching to the target user namespace in
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!
Exactly, so I was thinking of not having a securityfs_ns_create_mount
at all. All the securityfs_ns_create.. calls would be in the notifier
But we need to somehow have a call to get_tree_keyed() and have that
user namespace switched out. I don't know how else to do this other than
having some function that does that and that is now called
securityfs_ns_create_mount().
get_tree_keyed() will also call the fill_super() which is called when
securityfs_ns_create_mount() is called.
[ 196.739071] ima_fs_ns_init @ 639 before securityfs_ns_create_mount()
[ 196.740426] securityfs_init_fs_context @ 72 user_ns:
ffffffff98a3cc60; nr_extents: 1
[ 196.741519] securityfs_ns_create_mount @ 105 target user_ns:
ffff9e239753eb80; nr_extents: 0
[ 196.742657] securityfs_get_tree @ 60 before get_tree_keyed()
[ 196.743418] securityfs_fill_super @ 47 user_ns: ffff9e239753eb80;
nr_extents: 0
[ 196.744467] ima_fs_ns_init @ 641 after securityfs_ns_create_mount()
[ 196.745304] ima: Allocated hash algorithm: sha256
[ 196.757650] securityfs_init_fs_context @ 72 user_ns:
ffff9e239753eb80; nr_extents: 1
[ 196.758759] securityfs_get_tree @ 60 before get_tree_keyed()
You said it works by 'accident'. I know it works because the function
securityfs_init_fs_context() that now populates the filesystem via the
late_fs_init() is getting called twice. Does 'accident' here mean the
call sequence could change?
Where would the vfsmount pointer reside? For now it's in
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()?
exactly. I think struct user_namespace should have two elements gated
by a #ifdef CONFIG_SECURITYFS which are the vfsmount and the
mount_count for passing into simple_pin_fs.
Also that we can do for as long as it flies beyond the conversation
here... :-) Anyone else have an opinion ?
Stefan
James
