Messages in this thread |  | | Date | Wed, 8 Dec 2021 10:39:48 -0500 | Subject | Re: [PATCH v4 16/16] ima: Setup securityfs for IMA namespace | From | Stefan Berger <> |
| |
On 12/8/21 07:58, Christian Brauner wrote: > On Tue, Dec 07, 2021 at 03:21:27PM -0500, Stefan Berger wrote: >> >> #if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) >> diff --git a/security/inode.c b/security/inode.c >> index 121ac1874dde..10ee20917f42 100644 >> --- a/security/inode.c >> +++ b/security/inode.c >> @@ -16,6 +16,7 @@ >> #include <linux/fs_context.h> >> #include <linux/mount.h> >> #include <linux/pagemap.h> >> +#include <linux/ima.h> >> #include <linux/init.h> >> #include <linux/namei.h> >> #include <linux/security.h> >> @@ -41,6 +42,7 @@ static const struct super_operations securityfs_super_operations = { >> static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc) >> { >> static const struct tree_descr files[] = {{""}}; >> + struct user_namespace *ns = fc->user_ns; >> int error; >> >> error = simple_fill_super(sb, SECURITYFS_MAGIC, files); >> @@ -49,7 +51,10 @@ static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc) >> >> sb->s_op = &securityfs_super_operations; >> >> - return 0; >> + if (ns != &init_user_ns) >> + error = ima_fs_ns_init(ns, sb->s_root); >> + >> + return error; >> } >> >> static int securityfs_get_tree(struct fs_context *fc) >> @@ -69,6 +74,11 @@ static int securityfs_init_fs_context(struct fs_context *fc) >> >> static void securityfs_kill_super(struct super_block *sb) >> { >> + struct user_namespace *ns = sb->s_fs_info; >> + >> + if (ns != &init_user_ns) >> + ima_fs_ns_free_dentries(ns); > Say securityfs is unmounted. Then all the inodes and dentries become > invalid. It's not allowed to hold on to any dentries or inodes after the > super_block is shut down. So I just want to be sure that nothing in ima > can access these dentries after securityfs is unmounted.
> To put it another way: why are they stored in struct ima_namespace in > the first place? If you don't pin a filesystem when creating files or > directories like you do for securityfs in init_ima_ns then you don't > need to hold on to them as they will be automatically be wiped during > umount.
The reason was so that securityfs for init_ima_ns and IMA namespaces could share the code assigning to dentries to keep around and can clean up if an error occurs while creating a dentry.
What about this: We keep the dentries in the ima_namespace, modify the code creating the dentries in securityfs_create_dentry() to only take the additional reference in case of init_user_ns (I suppose this is what you suggest) and then keep 'static void ima_fs_ns_free_dentries()' only for removing the dentries for the error case and never call it from securityfs_kill_super()? Would that be acceptable?
|  |