Messages in this thread | | | Date | Tue, 7 Dec 2021 07:48:14 -0800 | Subject | Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns | From | Casey Schaufler <> |
| |
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.
| |