On 12/7/2021 7:40 AM, James Bottomley wrote:
On Tue, 2021-12-07 at 10:16 -0500, James Bottomley wrote:
On Tue, 2021-12-07 at 15:59 +0100, Christian Brauner wrote:
On Mon, Dec 06, 2021 at 04:14:15PM -0500, James Bottomley wrote:
[...]
static int securityfs_fill_super(struct super_block *sb, struct
fs_context *fc)
{
static const struct tree_descr files[] = {{""}};
int error;
+ struct user_namespace *ns = fc->user_ns;
error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
if (error)
return error;
+ ns->securityfs_root = dget(sb->s_root);
+
sb->s_op = &securityfs_super_operations;
+ if (ns != &init_user_ns)
+ blocking_notifier_call_chain(&securityfs_ns_notifier,
+ SECURITYFS_NS_ADD, ns);
I would propose not to use the notifier logic. While it might be
nifty it's over-engineered in my opinion.
The reason for a notifier is that this current patch set only
namespaces ima, but we also have integrity and evm to do. Plus, as
Casey said, we might get apparmour and selinux. Since each of those
will also want to add entries in fill_super, the notifier mechanism
seemed fairly tailor made for this. The alternative is to have a
load of
#if CONFIG_securityfeature
callback()
#endif
Inside securityfs_fill_super which is a bit inelegant.
The dentry stashing in struct user_namespace currently serves the
purpose to make it retrievable in ima_fs_ns_init(). That doesn't
justify its existence imho.
I can thread the root as part of the callback. I think I can still
use the standard securityfs calls because the only reason for the
dentry in the namespace is so the callee can pass NULL and have the
dentry created at the top level. We can insist in the namespaced use
case that the callee always pass in the dentry, even for the top
level.
There is one central place were all users of namespaced securityfs
can create the files that they need to and that is in
securityfs_fill_super(). (If you want to make that more obvious
then give it a subdirectory securityfs and move inode.c in there.)
Right, that's what the patch does.
We simply will expect users to add:
ima_init_securityfs()
mylsm_init_securityfs()
Yes, plus all the #ifdefs because securityfs can exist independently
of each of the features. We can hide the ifdefs in the header files
and make the functions static do nothing if not defined, but the
ifdeffery has to live somewhere.
Actually, I've got a much better reason: securityfs is a bool; all the
other LSMs and IMA are tristates. We can't call module init functions
from core code, it has to be done by something like a notifier.
Err, no. LSMs are not available as loadable modules.
