* include/linux/cgroup.h:553 suspicious rcu_dereference_check() usage!
@ 2012-09-14 9:23 Jiri Slaby
2012-09-14 9:40 ` [PATCH] cgroup: fix invalid rcu dereference Jiri Slaby
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2012-09-14 9:23 UTC (permalink / raw)
To: aris
Cc: Andrew Morton, Linux kernel mailing list, Tejun Heo, lizefan,
containers, cgroups, Jiri Slaby
Hi,
with current -next trees and LDEP enabled, I'm getting:
===============================
[ INFO: suspicious RCU usage. ]
3.6.0-rc5-next-20120913+ #42 Not tainted
-------------------------------
/home/latest/linux/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
It's due to the commit "device_cgroup: convert device_cgroup internally
to policy + exceptions". It 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.
regards,
--
js
suse labs
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] cgroup: fix invalid rcu dereference 2012-09-14 9:23 include/linux/cgroup.h:553 suspicious rcu_dereference_check() usage! Jiri Slaby @ 2012-09-14 9:40 ` Jiri Slaby 2012-09-14 13:27 ` Aristeu Rozanski 0 siblings, 1 reply; 5+ messages in thread From: Jiri Slaby @ 2012-09-14 9:40 UTC (permalink / raw) To: akpm; +Cc: jirislaby, linux-kernel, Jiri Slaby, aris, Tejun Heo, lizefan 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 ------------------------------- /home/latest/linux/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 Signed-off-by: Jiri Slaby <jslaby@suse.cz> Cc: aris@redhat.com Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Tejun Heo <tj@kernel.org> Cc: lizefan@huawei.com --- And this should fix it. security/device_cgroup.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 8d21ded..2178886 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -524,10 +524,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; @@ -538,6 +538,7 @@ static int __devcgroup_check_permission(struct dev_cgroup *dev_cgroup, ex.access = access; rcu_read_lock(); + dev_cgroup = task_devcgroup(current); rc = may_access(dev_cgroup, &ex); rcu_read_unlock(); @@ -549,7 +550,6 @@ static int __devcgroup_check_permission(struct dev_cgroup *dev_cgroup, 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)) @@ -561,13 +561,12 @@ int __devcgroup_inode_permission(struct inode *inode, int mask) 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)) @@ -578,7 +577,7 @@ int devcgroup_inode_mknod(int mode, dev_t dev) 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); } -- 1.7.11.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] cgroup: fix invalid rcu dereference 2012-09-14 9:40 ` [PATCH] cgroup: fix invalid rcu dereference Jiri Slaby @ 2012-09-14 13:27 ` Aristeu Rozanski 2012-09-14 17:01 ` Tejun Heo 0 siblings, 1 reply; 5+ messages in thread From: Aristeu Rozanski @ 2012-09-14 13:27 UTC (permalink / raw) To: Jiri Slaby; +Cc: akpm, jirislaby, linux-kernel, Tejun Heo, lizefan On Fri, Sep 14, 2012 at 11:40:12AM +0200, Jiri Slaby wrote: > 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. ugh, missed that > > 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 > ------------------------------- > /home/latest/linux/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 > > Signed-off-by: Jiri Slaby <jslaby@suse.cz> > Cc: aris@redhat.com > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Tejun Heo <tj@kernel.org> > Cc: lizefan@huawei.com > --- > > And this should fix it. > > security/device_cgroup.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/security/device_cgroup.c b/security/device_cgroup.c > index 8d21ded..2178886 100644 > --- a/security/device_cgroup.c > +++ b/security/device_cgroup.c > @@ -524,10 +524,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; > > @@ -538,6 +538,7 @@ static int __devcgroup_check_permission(struct dev_cgroup *dev_cgroup, > ex.access = access; > > rcu_read_lock(); > + dev_cgroup = task_devcgroup(current); > rc = may_access(dev_cgroup, &ex); > rcu_read_unlock(); > > @@ -549,7 +550,6 @@ static int __devcgroup_check_permission(struct dev_cgroup *dev_cgroup, > > 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)) > @@ -561,13 +561,12 @@ int __devcgroup_inode_permission(struct inode *inode, int mask) > 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)) > @@ -578,7 +577,7 @@ int devcgroup_inode_mknod(int mode, dev_t dev) > 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); > > } thanks Jiri Acked-by: Aristeu Rozanski <aris@redhat.com> -- Aristeu ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cgroup: fix invalid rcu dereference 2012-09-14 13:27 ` Aristeu Rozanski @ 2012-09-14 17:01 ` Tejun Heo 2012-09-17 1:09 ` Li Zefan 0 siblings, 1 reply; 5+ messages in thread From: Tejun Heo @ 2012-09-14 17:01 UTC (permalink / raw) To: Aristeu Rozanski; +Cc: Jiri Slaby, akpm, jirislaby, linux-kernel, lizefan On Fri, Sep 14, 2012 at 09:27:38AM -0400, Aristeu Rozanski wrote: > On Fri, Sep 14, 2012 at 11:40:12AM +0200, Jiri Slaby wrote: > > 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. > > ugh, missed that > ... > thanks Jiri > Acked-by: Aristeu Rozanski <aris@redhat.com> How should this be routed? Thanks. -- tejun ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cgroup: fix invalid rcu dereference 2012-09-14 17:01 ` Tejun Heo @ 2012-09-17 1:09 ` Li Zefan 0 siblings, 0 replies; 5+ messages in thread From: Li Zefan @ 2012-09-17 1:09 UTC (permalink / raw) To: Tejun Heo; +Cc: Aristeu Rozanski, Jiri Slaby, akpm, jirislaby, linux-kernel On 2012/9/15 1:01, Tejun Heo wrote: > On Fri, Sep 14, 2012 at 09:27:38AM -0400, Aristeu Rozanski wrote: >> On Fri, Sep 14, 2012 at 11:40:12AM +0200, Jiri Slaby wrote: >>> 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. >> >> ugh, missed that >> > ... >> thanks Jiri >> Acked-by: Aristeu Rozanski <aris@redhat.com> > > How should this be routed? > through akpm's -mm tree. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-17 1:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-14 9:23 include/linux/cgroup.h:553 suspicious rcu_dereference_check() usage! Jiri Slaby 2012-09-14 9:40 ` [PATCH] cgroup: fix invalid rcu dereference Jiri Slaby 2012-09-14 13:27 ` Aristeu Rozanski 2012-09-14 17:01 ` Tejun Heo 2012-09-17 1:09 ` Li Zefan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).