mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Aristeu Rozanski <aris@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Dave Jones <davej@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Tejun Heo <tj@kernel.org>, Li Zefan <lizefan@huawei.com>,
	James Morris <jmorris@namei.org>,
	Pavel Emelyanov <xemul@openvz.org>,
	Serge Hallyn <serge.hallyn@canonical.com>,
	Jiri Slaby <jslaby@suse.cz>,
	cgroups@vger.kernel.org
Subject: [PATCH 1/4] cgroup: fix invalid rcu dereference
Date: Mon, 22 Oct 2012 09:45:37 -0400	[thread overview]
Message-ID: <20121022134536.543579196@napanee.usersys.redhat.com> (raw)
In-Reply-To: <20121022134536.172969567@napanee.usersys.redhat.com>

[-- Attachment #1: rcu.patch --]
[-- Type: text/plain, Size: 3763 bytes --]

From: Jiri Slaby <jslaby@suse.cz>

Commit "device_cgroup: convert device_cgroup internally to policy +
exceptions" removed rcu locks which are needed in task_devcgroup
called in this chain: devcgroup_inode_mknod OR
__devcgroup_inode_permission -> __devcgroup_inode_permission ->
task_devcgroup -> task_subsys_state -> task_subsys_state_check.

Change the code so that task_devcgroup is safely called with rcu read
lock held.

===============================
[ INFO: suspicious RCU usage. ]
3.6.0-rc5-next-20120913+ #42 Not tainted
-------------------------------
include/linux/cgroup.h:553 suspicious rcu_dereference_check() usage!

other info that might help us debug this:

rcu_scheduler_active = 1, debug_locks = 0
2 locks held by kdevtmpfs/23:
 #0:  (sb_writers){.+.+.+}, at: [<ffffffff8116873f>]
mnt_want_write+0x1f/0x50
 #1:  (&sb->s_type->i_mutex_key#3/1){+.+.+.}, at: [<ffffffff811558af>]
kern_path_create+0x7f/0x170

stack backtrace:
Pid: 23, comm: kdevtmpfs Not tainted 3.6.0-rc5-next-20120913+ #42
Call Trace:
 [<ffffffff810c638d>] lockdep_rcu_suspicious+0xfd/0x130
 [<ffffffff8121541d>] devcgroup_inode_mknod+0x19d/0x240
 [<ffffffff8107bf54>] ? ns_capable+0x44/0x80
 [<ffffffff81156b21>] vfs_mknod+0x71/0xf0
 [<ffffffff813a8332>] handle_create.isra.2+0x72/0x200
 [<ffffffff813a85d4>] devtmpfsd+0x114/0x140
 [<ffffffff813a84c0>] ? handle_create.isra.2+0x200/0x200
 [<ffffffff81093ad6>] kthread+0xd6/0xe0
 [<ffffffff81654f24>] kernel_thread_helper+0x4/0x10
 [<ffffffff8165369d>] ? retint_restore_args+0xe/0xe
 [<ffffffff81093a00>] ? kthread_create_on_node+0x140/0x140
 [<ffffffff81654f20>] ? gs_change+0xb/0xb

Cc: Dave Jones <davej@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: James Morris <jmorris@namei.org>
Cc: Pavel Emelyanov <xemul@openvz.org>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>

---

And this should fix it.

 security/device_cgroup.c |   15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

--- github.orig/security/device_cgroup.c	2012-10-17 11:11:08.514793906 -0400
+++ github/security/device_cgroup.c	2012-10-19 16:35:37.936804289 -0400
@@ -533,10 +533,10 @@ struct cgroup_subsys devices_subsys = {
  *
  * returns 0 on success, -EPERM case the operation is not permitted
  */
-static int __devcgroup_check_permission(struct dev_cgroup *dev_cgroup,
-					short type, u32 major, u32 minor,
+static int __devcgroup_check_permission(short type, u32 major, u32 minor,
 				        short access)
 {
+	struct dev_cgroup *dev_cgroup;
 	struct dev_exception_item ex;
 	int rc;
 
@@ -547,6 +547,7 @@ 	memset(&ex, 0, sizeof(ex));
 	ex.access = access;
 
 	rcu_read_lock();
+	dev_cgroup = task_devcgroup(current);
 	rc = may_access(dev_cgroup, &ex);
 	rcu_read_unlock();
 
@@ -558,7 +559,6 @@ 	return 0;
 
 int __devcgroup_inode_permission(struct inode *inode, int mask)
 {
-	struct dev_cgroup *dev_cgroup = task_devcgroup(current);
 	short type, access = 0;
 
 	if (S_ISBLK(inode->i_mode))
@@ -570,13 +570,12 @@ 	short type, access = 0;
 	if (mask & MAY_READ)
 		access |= ACC_READ;
 
-	return __devcgroup_check_permission(dev_cgroup, type, imajor(inode),
-					    iminor(inode), access);
+	return __devcgroup_check_permission(type, imajor(inode), iminor(inode),
+			access);
 }
 
 int devcgroup_inode_mknod(int mode, dev_t dev)
 {
-	struct dev_cgroup *dev_cgroup = task_devcgroup(current);
 	short type;
 
 	if (!S_ISBLK(mode) && !S_ISCHR(mode))
@@ -587,7 +586,7 @@ 		return 0;
 	else
 		type = DEV_CHAR;
 
-	return __devcgroup_check_permission(dev_cgroup, type, MAJOR(dev),
-					    MINOR(dev), ACC_MKNOD);
+	return __devcgroup_check_permission(type, MAJOR(dev), MINOR(dev),
+			ACC_MKNOD);
 
 }


  reply	other threads:[~2012-10-22 13:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-22 13:45 [PATCH 0/4] Rebase device_cgroup v2 patchset Aristeu Rozanski
2012-10-22 13:45 ` Aristeu Rozanski [this message]
2012-10-22 16:11   ` [PATCH 1/4] cgroup: fix invalid rcu dereference Serge Hallyn
2012-10-23 12:50   ` Jiri Slaby
2012-10-23 13:17     ` Aristeu Rozanski
2012-10-23 13:53       ` Jiri Slaby
2012-10-22 13:45 ` [PATCH 2/4] device_cgroup: rename deny_all to behavior Aristeu Rozanski
2012-10-22 16:12   ` Serge Hallyn
2012-10-22 13:45 ` [PATCH 3/4] device_cgroup: stop using simple_strtoul() Aristeu Rozanski
2012-10-22 16:14   ` Serge Hallyn
2012-10-22 13:45 ` [PATCH 4/4] device_cgroup: add proper checking when changing default behavior Aristeu Rozanski
2012-10-22 16:16   ` Serge Hallyn
2012-10-22 19:58 ` [PATCH 0/4] Rebase device_cgroup v2 patchset Andrew Morton
2012-10-22 20:14   ` Aristeu Rozanski
2013-05-14 15:05 ` Serge Hallyn
2013-05-14 15:51   ` Aristeu Rozanski
2013-05-14 16:22     ` Serge Hallyn
2013-05-14 21:02       ` Eric W. Biederman
2013-05-16  1:14         ` Serge E. Hallyn
2013-05-16  1:23           ` Serge E. Hallyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121022134536.543579196@napanee.usersys.redhat.com \
    --to=aris@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=davej@redhat.com \
    --cc=jmorris@namei.org \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=serge.hallyn@canonical.com \
    --cc=tj@kernel.org \
    --cc=xemul@openvz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome