Re: [PATCH v4 11/16] securityfs: Only use simple_pin_fs/simple_release_fs for init_user_ns

Christian Brauner
2021-12-08T11:59:00Z

On Tue, Dec 07, 2021 at 03:21:22PM -0500, Stefan Berger wrote:
> To prepare for virtualization of SecurityFS, use simple_pin_fs and
> simpe_release_fs only when init_user_ns is active.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
> security/inode.c | 30 +++++++++++++++++++++---------
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/security/inode.c b/security/inode.c
> index 6c326939750d..1a720b2c566d 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -21,9 +21,10 @@
> #include <linux/security.h>
> #include <linux/lsm_hooks.h>
> #include <linux/magic.h>
> +#include <linux/user_namespace.h>
>
> -static struct vfsmount *mount;
> -static int mount_count;
> +static struct vfsmount *securityfs_mount;
> +static int securityfs_mount_count;
>
> static void securityfs_free_inode(struct inode *inode)
> {
> @@ -109,6 +110,7 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
> const struct file_operations *fops,
> const struct inode_operations *iops)
> {
> + struct user_namespace *ns = current_user_ns();
> struct dentry *dentry;
> struct inode *dir, *inode;
> int error;
> @@ -118,12 +120,17 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
>
> pr_debug("securityfs: creating file '%s'\n",name);
>
> - error = simple_pin_fs(&fs_type, &mount, &mount_count);
> - if (error)
> - return ERR_PTR(error);
> + if (ns == &init_user_ns) {
> + error = simple_pin_fs(&fs_type, &securityfs_mount,
> + &securityfs_mount_count);
> + if (error)
> + return ERR_PTR(error);
> + }
>
> - if (!parent)
> - parent = mount->mnt_root;
> + if (!parent) {
> + if (ns == &init_user_ns)
> + parent = securityfs_mount->mnt_root;

Wouldn't you want an

else
return ERR_PTR(-EINVAL);

in here already?

> + }
>
> dir = d_inode(parent);
>
> @@ -168,7 +175,9 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
> dentry = ERR_PTR(error);
> out:
> inode_unlock(dir);
> - simple_release_fs(&mount, &mount_count);
> + if (ns == &init_user_ns)
> + simple_release_fs(&securityfs_mount,
> + &securityfs_mount_count);
> return dentry;
> }
>
> @@ -294,6 +303,7 @@ EXPORT_SYMBOL_GPL(securityfs_create_symlink);
> */
> void securityfs_remove(struct dentry *dentry)
> {
> + struct user_namespace *ns = dentry->d_sb->s_user_ns;
> struct inode *dir;
>
> if (!dentry || IS_ERR(dentry))
> @@ -309,7 +319,9 @@ void securityfs_remove(struct dentry *dentry)
> dput(dentry);
> }
> inode_unlock(dir);
> - simple_release_fs(&mount, &mount_count);
> + if (ns == &init_user_ns)
> + simple_release_fs(&securityfs_mount,
> + &securityfs_mount_count);
> }
> EXPORT_SYMBOL_GPL(securityfs_remove);
>
> --
> 2.31.1
>
>