On Thu, 2021-12-09 at 15:37 +0100, Christian Brauner wrote: > On Thu, Dec 09, 2021 at 03:34:28PM +0100, Christian Brauner wrote: > > On Wed, Dec 08, 2021 at 05:18:17PM -0500, Stefan Berger wrote: > > > Move the dentries into the ima_namespace for reuse by virtualized > > > SecurityFS. Implement function freeing the dentries in order of > > > files and symlinks before directories. > > > > > > Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx> > > > --- > > > > This doesn't work as implemented, I think. > > > > What I would have preferred and what I tried to explain in the > > earlier review was: > > Keep the dentry stashing global since it is only needed for > > init_ima_ns. > > Then struct ima_namespace becomes way smaller and simpler. > > If you do that then it makes sense to remove the additional dget() > > in securityfs_create_dentry() for non-init_ima_ns. > > Then you can rely on auto-cleanup in .kill_sb() or on > > ima_securityfs_init() failure and you only need to call > > ima_fs_ns_free_dentries() if ns != init_ima_ns. > > > > IIuc, it seems you're currently doing one dput() too many since > > you're calling securityfs_remove() in the error path for non- > > init_ima_ns which relies on the previous increased dget() which we > > removed. > > If you really want to move the dentry stashing into struct > ima_namespace even though it's really unnecessary then you may as > well not care about the auto-cleanup and keep that additional > ima_fs_ns_free_dentries(ns) call in .kill_sb(). But I really think > not dragging dentry stashing into struct ima_namespace is the correct > way to go about this. We, unfortunately, do have one case we can't avoid stashing for the policy file. It's this code in ima_release_policy: > #if !defined(CONFIG_IMA_WRITE_POLICY) && > !defined(CONFIG_IMA_READ_POLICY) > securityfs_remove(ns->dentry[IMAFS_DENTRY_IMA_POLICY]); > ns->dentry[IMAFS_DENTRY_IMA_POLICY] = NULL; > What it does is that in certain config options, the policy file entry gets removed from the securityfs ima directory after you write to it. James
