* 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).