Re: [PATCH] selinux: selinux_setprocattr()->ptrace_parent() needs rcu_read_lock()

2013-12-16 Thread Paul Moore
On Saturday, December 14, 2013 05:32:56 PM Oleg Nesterov wrote: > On 12/14, Paul Moore wrote: > > ... I'm curious about the removal of the task lock; shouldn't we keep > > the task lock in place? > > Why? It protects nothing in this case, afaics. Unless of course it > protects cred->security someh

Re: [PATCH] selinux: selinux_setprocattr()->ptrace_parent() needs rcu_read_lock()

2013-12-14 Thread Oleg Nesterov
On 12/14, Paul Moore wrote: > > I understand your point, but I still think there is some value in > keeping the call to ptrace_parent() rather than fetching the ptrace > pointer on our own. Yes, agreed, I changed my mind ;) > However, that said, I think we should try and do something about the >

Re: [PATCH] selinux: selinux_setprocattr()->ptrace_parent() needs rcu_read_lock()

2013-12-14 Thread Paul Moore
On Fri, Dec 6, 2013 at 9:47 AM, Oleg Nesterov wrote: > On 12/05, Paul Moore wrote: >> On Thursday, December 05, 2013 05:59:53 PM Oleg Nesterov wrote: >> > >> > Note: perhaps we should simply kill ptrace_parent(), it buys >> > almost nothing and it is obviously racy. Or perhaps we should >> > chang

Re: [PATCH] selinux: selinux_setprocattr()->ptrace_parent() needs rcu_read_lock()

2013-12-06 Thread Oleg Nesterov
On 12/05, Paul Moore wrote: > > On Thursday, December 05, 2013 05:59:53 PM Oleg Nesterov wrote: > > > > Note: perhaps we should simply kill ptrace_parent(), it buys > > almost nothing and it is obviously racy. Or perhaps we should > > change it to ensure it can't wrongly return the natural parent >

Re: [PATCH] selinux: selinux_setprocattr()->ptrace_parent() needs rcu_read_lock()

2013-12-05 Thread Paul Moore
On Thursday, December 05, 2013 05:59:53 PM Oleg Nesterov wrote: > selinux_setprocattr() does ptrace_parent(p) under task_lock(p), > but task_struct->alloc_lock doesn't pin ->parent or ->ptrace, > this looks confusing and triggers the "suspicious RCU usage" > warning because ptrace_parent() does rcu

[PATCH] selinux: selinux_setprocattr()->ptrace_parent() needs rcu_read_lock()

2013-12-05 Thread Oleg Nesterov
selinux_setprocattr() does ptrace_parent(p) under task_lock(p), but task_struct->alloc_lock doesn't pin ->parent or ->ptrace, this looks confusing and triggers the "suspicious RCU usage" warning because ptrace_parent() does rcu_dereference_check(). And in theory this is wrong, spin_lock()->preempt