All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cgroupfs: use init_cred when populating new cgroupfs mount
@ 2011-05-25 21:35 ` Eric Paris
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Paris @ 2011-05-25 21:35 UTC (permalink / raw)
  To: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	selinux-+05T5uksL2qpZYMLLGbcSA
  Cc: sds-+05T5uksL2qpZYMLLGbcSA, menage-hpIqsD4AKlfQT0dZR+AlfA,
	rhel6-cc-external-list-H+wXaHxf7aLQT0dZR+AlfA

We recently found that in some configurations SELinux was blocking the ability
for cgroupfs to be mounted.  The reason for this is because cgroupfs creates
files and directories during the get_sb() call and also uses lookup_one_len()
during that same get_sb() call.  This is a problem since the security
subsystem cannot initialize the superblock and the inodes in that filesystem
until after the get_sb() call returns.  Thus we leave the inodes in
an unitialized state during get_sb().  For the vast majority of filesystems
this is not an issue, but since cgroupfs uses lookup_on_len() it does
search permission checks on the directories in the path it walks.  Since the
inode security state is not set up SELinux does these checks as if the inodes
were 'unlabeled.'

Many 'normal' userspace process do not have permission to interact with
unlabeled inodes.  The solution presented here is to do the permission checks
of path walk and inode creation as the kernel rather than as the task that
called mount.  Since the kernel has permission to read/write/create
unlabeled inodes the get_sb() call will complete successfully and the SELinux
code will be able to initialize the superblock and those inodes created during
the get_sb() call.

Signed-off-by: Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---

 kernel/cgroup.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 909a355..38b32dd 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -27,9 +27,11 @@
  */
 
 #include <linux/cgroup.h>
+#include <linux/cred.h>
 #include <linux/ctype.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
+#include <linux/init_task.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/mm.h>
@@ -1513,6 +1515,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		struct cgroup *root_cgrp = &root->top_cgroup;
 		struct inode *inode;
 		struct cgroupfs_root *existing_root;
+		const struct cred *cred;
 		int i;
 
 		BUG_ON(sb->s_root != NULL);
@@ -1592,7 +1595,9 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		BUG_ON(!list_empty(&root_cgrp->children));
 		BUG_ON(root->number_of_cgroups != 1);
 
+		cred = override_creds(&init_cred);
 		cgroup_populate_dir(root_cgrp);
+		revert_creds(cred);
 		mutex_unlock(&cgroup_mutex);
 		mutex_unlock(&inode->i_mutex);
 	} else {

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] cgroupfs: use init_cred when populating new cgroupfs mount
@ 2011-05-25 21:35 ` Eric Paris
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Paris @ 2011-05-25 21:35 UTC (permalink / raw)
  To: containers, selinux; +Cc: menage, lizf, sds, rhel6-cc-external-list

We recently found that in some configurations SELinux was blocking the ability
for cgroupfs to be mounted.  The reason for this is because cgroupfs creates
files and directories during the get_sb() call and also uses lookup_one_len()
during that same get_sb() call.  This is a problem since the security
subsystem cannot initialize the superblock and the inodes in that filesystem
until after the get_sb() call returns.  Thus we leave the inodes in
an unitialized state during get_sb().  For the vast majority of filesystems
this is not an issue, but since cgroupfs uses lookup_on_len() it does
search permission checks on the directories in the path it walks.  Since the
inode security state is not set up SELinux does these checks as if the inodes
were 'unlabeled.'

Many 'normal' userspace process do not have permission to interact with
unlabeled inodes.  The solution presented here is to do the permission checks
of path walk and inode creation as the kernel rather than as the task that
called mount.  Since the kernel has permission to read/write/create
unlabeled inodes the get_sb() call will complete successfully and the SELinux
code will be able to initialize the superblock and those inodes created during
the get_sb() call.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 kernel/cgroup.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 909a355..38b32dd 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -27,9 +27,11 @@
  */
 
 #include <linux/cgroup.h>
+#include <linux/cred.h>
 #include <linux/ctype.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
+#include <linux/init_task.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/mm.h>
@@ -1513,6 +1515,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		struct cgroup *root_cgrp = &root->top_cgroup;
 		struct inode *inode;
 		struct cgroupfs_root *existing_root;
+		const struct cred *cred;
 		int i;
 
 		BUG_ON(sb->s_root != NULL);
@@ -1592,7 +1595,9 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		BUG_ON(!list_empty(&root_cgrp->children));
 		BUG_ON(root->number_of_cgroups != 1);
 
+		cred = override_creds(&init_cred);
 		cgroup_populate_dir(root_cgrp);
+		revert_creds(cred);
 		mutex_unlock(&cgroup_mutex);
 		mutex_unlock(&inode->i_mutex);
 	} else {


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] cgroupfs: use init_cred when populating new cgroupfs mount
       [not found] ` <20110525213534.28016.33058.stgit-E+B5uJFuEZf0UfVguI6niVaTQe2KTcn/@public.gmane.org>
@ 2011-05-25 22:16   ` Paul Menage
  2011-05-26  0:57     ` Casey Schaufler
  1 sibling, 0 replies; 5+ messages in thread
From: Paul Menage @ 2011-05-25 22:16 UTC (permalink / raw)
  To: Eric Paris
  Cc: Stephen Smalley, Linux Containers,
	rhel6-cc-external-list-H+wXaHxf7aLQT0dZR+AlfA,
	selinux-+05T5uksL2qpZYMLLGbcSA

On Wed, May 25, 2011 at 2:35 PM, Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> Signed-off-by: Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Acked-by: Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Thanks,
Paul

> ---
>
>  kernel/cgroup.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 909a355..38b32dd 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -27,9 +27,11 @@
>  */
>
>  #include <linux/cgroup.h>
> +#include <linux/cred.h>
>  #include <linux/ctype.h>
>  #include <linux/errno.h>
>  #include <linux/fs.h>
> +#include <linux/init_task.h>
>  #include <linux/kernel.h>
>  #include <linux/list.h>
>  #include <linux/mm.h>
> @@ -1513,6 +1515,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
>                struct cgroup *root_cgrp = &root->top_cgroup;
>                struct inode *inode;
>                struct cgroupfs_root *existing_root;
> +               const struct cred *cred;
>                int i;
>
>                BUG_ON(sb->s_root != NULL);
> @@ -1592,7 +1595,9 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
>                BUG_ON(!list_empty(&root_cgrp->children));
>                BUG_ON(root->number_of_cgroups != 1);
>
> +               cred = override_creds(&init_cred);
>                cgroup_populate_dir(root_cgrp);
> +               revert_creds(cred);
>                mutex_unlock(&cgroup_mutex);
>                mutex_unlock(&inode->i_mutex);
>        } else {
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cgroupfs: use init_cred when populating new cgroupfs mount
  2011-05-25 21:35 ` Eric Paris
@ 2011-05-26  0:57     ` Casey Schaufler
  -1 siblings, 0 replies; 5+ messages in thread
From: Casey Schaufler @ 2011-05-26  0:57 UTC (permalink / raw)
  To: Eric Paris
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	selinux-+05T5uksL2qpZYMLLGbcSA, menage-hpIqsD4AKlfQT0dZR+AlfA,
	rhel6-cc-external-list-H+wXaHxf7aLQT0dZR+AlfA,
	sds-+05T5uksL2qpZYMLLGbcSA

On 5/25/2011 2:35 PM, Eric Paris wrote:
> We recently found that in some configurations SELinux was blocking the ability
> for cgroupfs to be mounted.  The reason for this is because cgroupfs creates
> files and directories during the get_sb() call and also uses lookup_one_len()
> during that same get_sb() call.  This is a problem since the security
> subsystem cannot initialize the superblock and the inodes in that filesystem
> until after the get_sb() call returns.  Thus we leave the inodes in
> an unitialized state during get_sb().  For the vast majority of filesystems
> this is not an issue, but since cgroupfs uses lookup_on_len() it does
> search permission checks on the directories in the path it walks.  Since the
> inode security state is not set up SELinux does these checks as if the inodes
> were 'unlabeled.'
>
> Many 'normal' userspace process do not have permission to interact with
> unlabeled inodes.  The solution presented here is to do the permission checks
> of path walk and inode creation as the kernel rather than as the task that
> called mount.  Since the kernel has permission to read/write/create
> unlabeled inodes the get_sb() call will complete successfully and the SELinux
> code will be able to initialize the superblock and those inodes created during
> the get_sb() call.
>
> Signed-off-by: Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Any idea what this change will do to the behavior of other LSMs?
In any case, it should be posted to the LSM list.


> ---
>
>  kernel/cgroup.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 909a355..38b32dd 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -27,9 +27,11 @@
>   */
>  
>  #include <linux/cgroup.h>
> +#include <linux/cred.h>
>  #include <linux/ctype.h>
>  #include <linux/errno.h>
>  #include <linux/fs.h>
> +#include <linux/init_task.h>
>  #include <linux/kernel.h>
>  #include <linux/list.h>
>  #include <linux/mm.h>
> @@ -1513,6 +1515,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
>  		struct cgroup *root_cgrp = &root->top_cgroup;
>  		struct inode *inode;
>  		struct cgroupfs_root *existing_root;
> +		const struct cred *cred;
>  		int i;
>  
>  		BUG_ON(sb->s_root != NULL);
> @@ -1592,7 +1595,9 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
>  		BUG_ON(!list_empty(&root_cgrp->children));
>  		BUG_ON(root->number_of_cgroups != 1);
>  
> +		cred = override_creds(&init_cred);
>  		cgroup_populate_dir(root_cgrp);
> +		revert_creds(cred);
>  		mutex_unlock(&cgroup_mutex);
>  		mutex_unlock(&inode->i_mutex);
>  	} else {
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo-+05T5uksL2qpZYMLLGbcSA@public.gmane.org with
> the words "unsubscribe selinux" without quotes as the message.
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cgroupfs: use init_cred when populating new cgroupfs mount
@ 2011-05-26  0:57     ` Casey Schaufler
  0 siblings, 0 replies; 5+ messages in thread
From: Casey Schaufler @ 2011-05-26  0:57 UTC (permalink / raw)
  To: Eric Paris; +Cc: containers, selinux, menage, lizf, sds, rhel6-cc-external-list

On 5/25/2011 2:35 PM, Eric Paris wrote:
> We recently found that in some configurations SELinux was blocking the ability
> for cgroupfs to be mounted.  The reason for this is because cgroupfs creates
> files and directories during the get_sb() call and also uses lookup_one_len()
> during that same get_sb() call.  This is a problem since the security
> subsystem cannot initialize the superblock and the inodes in that filesystem
> until after the get_sb() call returns.  Thus we leave the inodes in
> an unitialized state during get_sb().  For the vast majority of filesystems
> this is not an issue, but since cgroupfs uses lookup_on_len() it does
> search permission checks on the directories in the path it walks.  Since the
> inode security state is not set up SELinux does these checks as if the inodes
> were 'unlabeled.'
>
> Many 'normal' userspace process do not have permission to interact with
> unlabeled inodes.  The solution presented here is to do the permission checks
> of path walk and inode creation as the kernel rather than as the task that
> called mount.  Since the kernel has permission to read/write/create
> unlabeled inodes the get_sb() call will complete successfully and the SELinux
> code will be able to initialize the superblock and those inodes created during
> the get_sb() call.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>

Any idea what this change will do to the behavior of other LSMs?
In any case, it should be posted to the LSM list.


> ---
>
>  kernel/cgroup.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 909a355..38b32dd 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -27,9 +27,11 @@
>   */
>  
>  #include <linux/cgroup.h>
> +#include <linux/cred.h>
>  #include <linux/ctype.h>
>  #include <linux/errno.h>
>  #include <linux/fs.h>
> +#include <linux/init_task.h>
>  #include <linux/kernel.h>
>  #include <linux/list.h>
>  #include <linux/mm.h>
> @@ -1513,6 +1515,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
>  		struct cgroup *root_cgrp = &root->top_cgroup;
>  		struct inode *inode;
>  		struct cgroupfs_root *existing_root;
> +		const struct cred *cred;
>  		int i;
>  
>  		BUG_ON(sb->s_root != NULL);
> @@ -1592,7 +1595,9 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
>  		BUG_ON(!list_empty(&root_cgrp->children));
>  		BUG_ON(root->number_of_cgroups != 1);
>  
> +		cred = override_creds(&init_cred);
>  		cgroup_populate_dir(root_cgrp);
> +		revert_creds(cred);
>  		mutex_unlock(&cgroup_mutex);
>  		mutex_unlock(&inode->i_mutex);
>  	} else {
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
>
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-05-26  0:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-25 21:35 [PATCH] cgroupfs: use init_cred when populating new cgroupfs mount Eric Paris
2011-05-25 21:35 ` Eric Paris
     [not found] ` <20110525213534.28016.33058.stgit-E+B5uJFuEZf0UfVguI6niVaTQe2KTcn/@public.gmane.org>
2011-05-25 22:16   ` Paul Menage
2011-05-26  0:57   ` Casey Schaufler
2011-05-26  0:57     ` Casey Schaufler

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.