| Message ID | 20211207202127.1508689-12-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima: Namespace IMA with audit support in IMA-ns | expand |
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 > >
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; Maybe better: static struct vfsmount *init_securityfs_mount; static int init_securityfs_mount_count; gets a bit long but gets the meaning across better plus it's a global so not really an issue imho if it's long. Otherwise, looks good. Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
On 12/8/21 06:58, Christian Brauner wrote: > 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? Fixed.
On Tue, 2021-12-07 at 15:21 -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> What do you mean by virtualization, and how does this prepare securityfs for it? The commit message should be way more verbose. /Jarkko
On Sat, 2021-12-11 at 16:16 +0200, Jarkko Sakkinen wrote: > On Tue, 2021-12-07 at 15:21 -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> > > What do you mean by virtualization, and how does this prepare > securityfs for it? The commit message should be way more verbose. Heh, well cart before horse: we're still trying to work out how to do it correctly, so we can't really document it until we've figured that bit out. Once that's all sorted, the output is likely something in Documentation/ explaining how to namespace a pseudo filesystem (since we have quite a few of them in the kernel) rather than a commit message which will get hard to find the next time someone wants to do this. James
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; + } 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);