On Wed, Dec 08, 2021 at 09:11:09AM -0500, James Bottomley wrote:

On Wed, 2021-12-08 at 13:58 +0100, Christian Brauner wrote:

quoted

On Tue, Dec 07, 2021 at 03:21:27PM -0500, Stefan Berger wrote:

[...]

quoted

quoted

@@ -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.

For IMA this is true because IMA can't be a module.  However, a modular

This thread is about ima and its stashing of dentries in struct
ima_namespace. That things might be different for other consumers is
uninteresting for this specific case, I think.

consumer, like the TPM, must be able to remove its entries from a
mounted securityfs because the code that serves the operations is going
away.  In order to do this removal, it needs the dentries somewhere. 

That still doesn't require you to take an additional reference on the
dentry per se.
Aside from this brings in a whole different and way bigger issue as that
requires way more fundamental work since this is about a (pseudo or
proper) device. It's not even clear that this should have entries
outside of init_user_ns-securityfs.

The current convention seems to be everything has a directory in the
top level, so we could call d_genocide() on this directory and not have
to worry about storing the dentries underneath, but I think we can't
avoid storing the dentry for the top level directory.

I have not heard an argument why ima needs to stash these dentries as it
doesn't remove them once created until umount.