All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
@ 2011-12-13 14:49 David Howells
  2011-12-13 23:11 ` James Morris
  2011-12-16 13:13 ` David Howells
  0 siblings, 2 replies; 18+ messages in thread
From: David Howells @ 2011-12-13 14:49 UTC (permalink / raw)
  To: jmorris, linux-security-module
  Cc: selinux, David Howells, Paul Moore, Eric Dumazet

Fix the following bug in sel_netport_insert() where rcu_dereference() should
be rcu_dereference_protected() as sel_netport_lock is held.

===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
security/selinux/netport.c:127 invoked rcu_dereference_check() without protection!

other info that might help us debug this:


rcu_scheduler_active = 1, debug_locks = 0
1 lock held by ossec-rootcheck/3323:
 #0:  (sel_netport_lock){+.....}, at: [<ffffffff8117d775>] sel_netport_sid+0xbb/0x226

stack backtrace:
Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
Call Trace:
 [<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
 [<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
 [<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
 [<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
 [<ffffffff810a5388>] ? might_fault+0x4e/0x9e
 [<ffffffff810a53d1>] ? might_fault+0x97/0x9e
 [<ffffffff81171cf4>] security_socket_bind+0x11/0x13
 [<ffffffff812ba967>] sys_bind+0x56/0x95
 [<ffffffff81380dac>] ? sysret_check+0x27/0x62
 [<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
 [<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
 [<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
 [<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
---

 security/selinux/netport.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/security/selinux/netport.c b/security/selinux/netport.c
index 0b62bd1..7b9eb1f 100644
--- a/security/selinux/netport.c
+++ b/security/selinux/netport.c
@@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
 	if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
 		struct sel_netport *tail;
 		tail = list_entry(
-			rcu_dereference(sel_netport_hash[idx].list.prev),
+			rcu_dereference_protected(
+				sel_netport_hash[idx].list.prev,
+				lockdep_is_held(&sel_netport_lock)),
 			struct sel_netport, list);
 		list_del_rcu(&tail->list);
 		kfree_rcu(tail, rcu);


--
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] 18+ messages in thread

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
  2011-12-13 14:49 [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert() David Howells
@ 2011-12-13 23:11 ` James Morris
  2011-12-16 13:13 ` David Howells
  1 sibling, 0 replies; 18+ messages in thread
From: James Morris @ 2011-12-13 23:11 UTC (permalink / raw)
  To: David Howells; +Cc: linux-security-module, selinux, Paul Moore, Eric Dumazet

On Tue, 13 Dec 2011, David Howells wrote:

> Fix the following bug in sel_netport_insert() where rcu_dereference() should
> be rcu_dereference_protected() as sel_netport_lock is held.
> 

Which kernel should this go to?


> ===================================================
> [ INFO: suspicious rcu_dereference_check() usage. ]
> ---------------------------------------------------
> security/selinux/netport.c:127 invoked rcu_dereference_check() without protection!
> 
> other info that might help us debug this:
> 
> 
> rcu_scheduler_active = 1, debug_locks = 0
> 1 lock held by ossec-rootcheck/3323:
>  #0:  (sel_netport_lock){+.....}, at: [<ffffffff8117d775>] sel_netport_sid+0xbb/0x226
> 
> stack backtrace:
> Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
> Call Trace:
>  [<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
>  [<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
>  [<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
>  [<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
>  [<ffffffff810a5388>] ? might_fault+0x4e/0x9e
>  [<ffffffff810a53d1>] ? might_fault+0x97/0x9e
>  [<ffffffff81171cf4>] security_socket_bind+0x11/0x13
>  [<ffffffff812ba967>] sys_bind+0x56/0x95
>  [<ffffffff81380dac>] ? sysret_check+0x27/0x62
>  [<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
>  [<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
>  [<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>  [<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> Acked-by: Paul Moore <paul@paul-moore.com>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> 
>  security/selinux/netport.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/security/selinux/netport.c b/security/selinux/netport.c
> index 0b62bd1..7b9eb1f 100644
> --- a/security/selinux/netport.c
> +++ b/security/selinux/netport.c
> @@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
>  	if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
>  		struct sel_netport *tail;
>  		tail = list_entry(
> -			rcu_dereference(sel_netport_hash[idx].list.prev),
> +			rcu_dereference_protected(
> +				sel_netport_hash[idx].list.prev,
> +				lockdep_is_held(&sel_netport_lock)),
>  			struct sel_netport, list);
>  		list_del_rcu(&tail->list);
>  		kfree_rcu(tail, rcu);
> 

-- 
James Morris
<jmorris@namei.org>

--
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] 18+ messages in thread

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
  2011-12-13 14:49 [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert() David Howells
  2011-12-13 23:11 ` James Morris
@ 2011-12-16 13:13 ` David Howells
  1 sibling, 0 replies; 18+ messages in thread
From: David Howells @ 2011-12-16 13:13 UTC (permalink / raw)
  To: James Morris
  Cc: dhowells, linux-security-module, selinux, Paul Moore, Eric Dumazet

James Morris <jmorris@namei.org> wrote:

> On Tue, 13 Dec 2011, David Howells wrote:
> 
> > Fix the following bug in sel_netport_insert() where rcu_dereference() should
> > be rcu_dereference_protected() as sel_netport_lock is held.
> > 
> 
> Which kernel should this go to?

I'm applying it to Linus's git head.  It probably needs to go to some stable
trees too, I suppose.

David

--
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] 18+ messages in thread

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
  2011-10-05 13:32   ` David Howells
@ 2011-10-06 22:51     ` Paul Moore
  -1 siblings, 0 replies; 18+ messages in thread
From: Paul Moore @ 2011-10-06 22:51 UTC (permalink / raw)
  To: David Howells; +Cc: selinux, netdev

On Wednesday, October 05, 2011 02:32:03 PM David Howells wrote:
> Paul Moore <paul@paul-moore.com> wrote:
> > We should probably do the same for the security/selinux/netif.c as it
> > uses the same logic; David is this something you want to tackle?
> 
> netif.c doesn't use any rcu_dereference*() function directly, though it does
> use list_for_each_entry_rcu().  However, I'm not sure that's a problem. 
> What is it you're referring to?

My apologies, the netport.c and netif.c code is very, very similar and 
whenever I see a patch just for one of the two it causes a reaction that you 
saw above.  While netif.c has a similar function, sel_netif_insert(), it is 
slightly different and doesn't need a rcu_dereference() ad the netport.c code 
does.

Sorry for the confusion.

-- 
paul moore
www.paul-moore.com

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

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
@ 2011-10-06 22:51     ` Paul Moore
  0 siblings, 0 replies; 18+ messages in thread
From: Paul Moore @ 2011-10-06 22:51 UTC (permalink / raw)
  To: David Howells; +Cc: selinux, netdev

On Wednesday, October 05, 2011 02:32:03 PM David Howells wrote:
> Paul Moore <paul@paul-moore.com> wrote:
> > We should probably do the same for the security/selinux/netif.c as it
> > uses the same logic; David is this something you want to tackle?
> 
> netif.c doesn't use any rcu_dereference*() function directly, though it does
> use list_for_each_entry_rcu().  However, I'm not sure that's a problem. 
> What is it you're referring to?

My apologies, the netport.c and netif.c code is very, very similar and 
whenever I see a patch just for one of the two it causes a reaction that you 
saw above.  While netif.c has a similar function, sel_netif_insert(), it is 
slightly different and doesn't need a rcu_dereference() ad the netport.c code 
does.

Sorry for the confusion.

-- 
paul moore
www.paul-moore.com


--
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] 18+ messages in thread

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
  2011-10-03 13:58 ` David Howells
@ 2011-10-05 13:32   ` David Howells
  -1 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2011-10-05 13:32 UTC (permalink / raw)
  To: Paul Moore; +Cc: dhowells, selinux, netdev

Paul Moore <paul@paul-moore.com> wrote:

> We should probably do the same for the security/selinux/netif.c as it uses
> the same logic; David is this something you want to tackle?

netif.c doesn't use any rcu_dereference*() function directly, though it does
use list_for_each_entry_rcu().  However, I'm not sure that's a problem.  What
is it you're referring to?

David

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

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
@ 2011-10-05 13:32   ` David Howells
  0 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2011-10-05 13:32 UTC (permalink / raw)
  To: Paul Moore; +Cc: dhowells, selinux, netdev

Paul Moore <paul@paul-moore.com> wrote:

> We should probably do the same for the security/selinux/netif.c as it uses
> the same logic; David is this something you want to tackle?

netif.c doesn't use any rcu_dereference*() function directly, though it does
use list_for_each_entry_rcu().  However, I'm not sure that's a problem.  What
is it you're referring to?

David

--
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] 18+ messages in thread

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
  2011-10-03 21:30   ` Paul Moore
@ 2011-10-05 11:07     ` David Howells
  -1 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2011-10-05 11:07 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: dhowells, Paul Moore, selinux, netdev

Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Usual way is to use :
> 	rcu_dereference_protected(
> 		sel_netport_hash[idx].list.prev,
> 		lockdep_is_held(&sel_netport_lock)),

Good point.

David

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

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
@ 2011-10-05 11:07     ` David Howells
  0 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2011-10-05 11:07 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: dhowells, Paul Moore, selinux, netdev

Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Usual way is to use :
> 	rcu_dereference_protected(
> 		sel_netport_hash[idx].list.prev,
> 		lockdep_is_held(&sel_netport_lock)),

Good point.

David

--
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] 18+ messages in thread

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
  2011-10-03 21:30   ` Paul Moore
  (?)
@ 2011-10-04  4:22   ` Eric Dumazet
  -1 siblings, 0 replies; 18+ messages in thread
From: Eric Dumazet @ 2011-10-04  4:22 UTC (permalink / raw)
  To: Paul Moore; +Cc: David Howells, selinux, netdev

Le lundi 03 octobre 2011 à 17:30 -0400, Paul Moore a écrit :
> On Monday, October 03, 2011 02:58:24 PM David Howells wrote:
> > Fix the following bug in sel_netport_insert() where rcu_dereference() should
> > be rcu_dereference_protected() as sel_netport_lock is held.
> > 
> > ===================================================
> > [ INFO: suspicious rcu_dereference_check() usage. ]
> > ---------------------------------------------------
> > security/selinux/netport.c:127 invoked rcu_dereference_check() without
> > protection!
> > 
> > other info that might help us debug this:
> > 
> > 
> > rcu_scheduler_active = 1, debug_locks = 0
> > 1 lock held by ossec-rootcheck/3323:
> >  #0:  (sel_netport_lock){+.....}, at: [<ffffffff8117d775>]
> > sel_netport_sid+0xbb/0x226
> > 
> > stack backtrace:
> > Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
> > Call Trace:
> >  [<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
> >  [<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
> >  [<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
> >  [<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
> >  [<ffffffff810a5388>] ? might_fault+0x4e/0x9e
> >  [<ffffffff810a53d1>] ? might_fault+0x97/0x9e
> >  [<ffffffff81171cf4>] security_socket_bind+0x11/0x13
> >  [<ffffffff812ba967>] sys_bind+0x56/0x95
> >  [<ffffffff81380dac>] ? sysret_check+0x27/0x62
> >  [<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
> >  [<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
> >  [<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> >  [<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b
> > 
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > ---
> > 
> >  security/selinux/netport.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> We should probably do the same for the security/selinux/netif.c as it uses the 
> same logic; David is this something you want to tackle?
> 
> Acked-by: Paul Moore <paul@paul-moore.com>
> 
> > diff --git a/security/selinux/netport.c b/security/selinux/netport.c
> > index 0b62bd1..39e2138 100644
> > --- a/security/selinux/netport.c
> > +++ b/security/selinux/netport.c
> > @@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
> > if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
> >  		struct sel_netport *tail;
> >  		tail = list_entry(
> > -			rcu_dereference(sel_netport_hash[idx].list.prev),
> > +			rcu_dereference_protected(
> > +				sel_netport_hash[idx].list.prev,
> > +				spin_is_locked(&sel_netport_lock)),

Usual way is to use :
	rcu_dereference_protected(
		sel_netport_hash[idx].list.prev,
		lockdep_is_held(&sel_netport_lock)),

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

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
  2011-10-03 23:07   ` David Howells
@ 2011-10-04  0:06     ` Paul Moore
  -1 siblings, 0 replies; 18+ messages in thread
From: Paul Moore @ 2011-10-04  0:06 UTC (permalink / raw)
  To: David Howells; +Cc: selinux, netdev

On Tuesday, October 4, 2011 12:07:42 AM David Howells wrote:
> Paul Moore <paul@paul-moore.com> wrote:
> > We should probably do the same for the security/selinux/netif.c as it
> > uses the same logic; David is this something you want to tackle?
> 
> I can have a look, but it won't be before Wednesday.

Not a problem, if you don't get to it just let me know and I'll put together a 
patch.

Thanks.

-- 
paul moore
www.paul-moore.com

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

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
@ 2011-10-04  0:06     ` Paul Moore
  0 siblings, 0 replies; 18+ messages in thread
From: Paul Moore @ 2011-10-04  0:06 UTC (permalink / raw)
  To: David Howells; +Cc: selinux, netdev

On Tuesday, October 4, 2011 12:07:42 AM David Howells wrote:
> Paul Moore <paul@paul-moore.com> wrote:
> > We should probably do the same for the security/selinux/netif.c as it
> > uses the same logic; David is this something you want to tackle?
> 
> I can have a look, but it won't be before Wednesday.

Not a problem, if you don't get to it just let me know and I'll put together a 
patch.

Thanks.

-- 
paul moore
www.paul-moore.com


--
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] 18+ messages in thread

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
  2011-10-03 13:58 ` David Howells
@ 2011-10-03 23:07   ` David Howells
  -1 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2011-10-03 23:07 UTC (permalink / raw)
  To: Paul Moore
  Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA, selinux-+05T5uksL2qpZYMLLGbcSA,
	netdev-u79uwXL29TY76Z2rM5mHXA

Paul Moore <paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org> wrote:

> We should probably do the same for the security/selinux/netif.c as it uses
> the same logic; David is this something you want to tackle?

I can have a look, but it won't be before Wednesday.

David

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

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
@ 2011-10-03 23:07   ` David Howells
  0 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2011-10-03 23:07 UTC (permalink / raw)
  To: Paul Moore; +Cc: dhowells, selinux, netdev

Paul Moore <paul@paul-moore.com> wrote:

> We should probably do the same for the security/selinux/netif.c as it uses
> the same logic; David is this something you want to tackle?

I can have a look, but it won't be before Wednesday.

David

--
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] 18+ messages in thread

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
  2011-10-03 13:58 ` David Howells
@ 2011-10-03 21:30   ` Paul Moore
  -1 siblings, 0 replies; 18+ messages in thread
From: Paul Moore @ 2011-10-03 21:30 UTC (permalink / raw)
  To: David Howells; +Cc: selinux, netdev

On Monday, October 03, 2011 02:58:24 PM David Howells wrote:
> Fix the following bug in sel_netport_insert() where rcu_dereference() should
> be rcu_dereference_protected() as sel_netport_lock is held.
> 
> ===================================================
> [ INFO: suspicious rcu_dereference_check() usage. ]
> ---------------------------------------------------
> security/selinux/netport.c:127 invoked rcu_dereference_check() without
> protection!
> 
> other info that might help us debug this:
> 
> 
> rcu_scheduler_active = 1, debug_locks = 0
> 1 lock held by ossec-rootcheck/3323:
>  #0:  (sel_netport_lock){+.....}, at: [<ffffffff8117d775>]
> sel_netport_sid+0xbb/0x226
> 
> stack backtrace:
> Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
> Call Trace:
>  [<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
>  [<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
>  [<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
>  [<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
>  [<ffffffff810a5388>] ? might_fault+0x4e/0x9e
>  [<ffffffff810a53d1>] ? might_fault+0x97/0x9e
>  [<ffffffff81171cf4>] security_socket_bind+0x11/0x13
>  [<ffffffff812ba967>] sys_bind+0x56/0x95
>  [<ffffffff81380dac>] ? sysret_check+0x27/0x62
>  [<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
>  [<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
>  [<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>  [<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  security/selinux/netport.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)

We should probably do the same for the security/selinux/netif.c as it uses the 
same logic; David is this something you want to tackle?

Acked-by: Paul Moore <paul@paul-moore.com>

> diff --git a/security/selinux/netport.c b/security/selinux/netport.c
> index 0b62bd1..39e2138 100644
> --- a/security/selinux/netport.c
> +++ b/security/selinux/netport.c
> @@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
> if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
>  		struct sel_netport *tail;
>  		tail = list_entry(
> -			rcu_dereference(sel_netport_hash[idx].list.prev),
> +			rcu_dereference_protected(
> +				sel_netport_hash[idx].list.prev,
> +				spin_is_locked(&sel_netport_lock)),
>  			struct sel_netport, list);
>  		list_del_rcu(&tail->list);
>  		kfree_rcu(tail, rcu);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
paul moore
www.paul-moore.com

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

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
@ 2011-10-03 21:30   ` Paul Moore
  0 siblings, 0 replies; 18+ messages in thread
From: Paul Moore @ 2011-10-03 21:30 UTC (permalink / raw)
  To: David Howells; +Cc: selinux, netdev

On Monday, October 03, 2011 02:58:24 PM David Howells wrote:
> Fix the following bug in sel_netport_insert() where rcu_dereference() should
> be rcu_dereference_protected() as sel_netport_lock is held.
> 
> ===================================================
> [ INFO: suspicious rcu_dereference_check() usage. ]
> ---------------------------------------------------
> security/selinux/netport.c:127 invoked rcu_dereference_check() without
> protection!
> 
> other info that might help us debug this:
> 
> 
> rcu_scheduler_active = 1, debug_locks = 0
> 1 lock held by ossec-rootcheck/3323:
>  #0:  (sel_netport_lock){+.....}, at: [<ffffffff8117d775>]
> sel_netport_sid+0xbb/0x226
> 
> stack backtrace:
> Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
> Call Trace:
>  [<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
>  [<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
>  [<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
>  [<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
>  [<ffffffff810a5388>] ? might_fault+0x4e/0x9e
>  [<ffffffff810a53d1>] ? might_fault+0x97/0x9e
>  [<ffffffff81171cf4>] security_socket_bind+0x11/0x13
>  [<ffffffff812ba967>] sys_bind+0x56/0x95
>  [<ffffffff81380dac>] ? sysret_check+0x27/0x62
>  [<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
>  [<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
>  [<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>  [<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  security/selinux/netport.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)

We should probably do the same for the security/selinux/netif.c as it uses the 
same logic; David is this something you want to tackle?

Acked-by: Paul Moore <paul@paul-moore.com>

> diff --git a/security/selinux/netport.c b/security/selinux/netport.c
> index 0b62bd1..39e2138 100644
> --- a/security/selinux/netport.c
> +++ b/security/selinux/netport.c
> @@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
> if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
>  		struct sel_netport *tail;
>  		tail = list_entry(
> -			rcu_dereference(sel_netport_hash[idx].list.prev),
> +			rcu_dereference_protected(
> +				sel_netport_hash[idx].list.prev,
> +				spin_is_locked(&sel_netport_lock)),
>  			struct sel_netport, list);
>  		list_del_rcu(&tail->list);
>  		kfree_rcu(tail, rcu);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
paul moore
www.paul-moore.com


--
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] 18+ messages in thread

* [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
@ 2011-10-03 13:58 ` David Howells
  0 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2011-10-03 13:58 UTC (permalink / raw)
  To: selinux; +Cc: netdev, dhowells

Fix the following bug in sel_netport_insert() where rcu_dereference() should
be rcu_dereference_protected() as sel_netport_lock is held.

===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
security/selinux/netport.c:127 invoked rcu_dereference_check() without protection!

other info that might help us debug this:


rcu_scheduler_active = 1, debug_locks = 0
1 lock held by ossec-rootcheck/3323:
 #0:  (sel_netport_lock){+.....}, at: [<ffffffff8117d775>] sel_netport_sid+0xbb/0x226

stack backtrace:
Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
Call Trace:
 [<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
 [<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
 [<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
 [<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
 [<ffffffff810a5388>] ? might_fault+0x4e/0x9e
 [<ffffffff810a53d1>] ? might_fault+0x97/0x9e
 [<ffffffff81171cf4>] security_socket_bind+0x11/0x13
 [<ffffffff812ba967>] sys_bind+0x56/0x95
 [<ffffffff81380dac>] ? sysret_check+0x27/0x62
 [<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
 [<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
 [<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
 [<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b

Signed-off-by: David Howells <dhowells@redhat.com>
---

 security/selinux/netport.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/security/selinux/netport.c b/security/selinux/netport.c
index 0b62bd1..39e2138 100644
--- a/security/selinux/netport.c
+++ b/security/selinux/netport.c
@@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
 	if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
 		struct sel_netport *tail;
 		tail = list_entry(
-			rcu_dereference(sel_netport_hash[idx].list.prev),
+			rcu_dereference_protected(
+				sel_netport_hash[idx].list.prev,
+				spin_is_locked(&sel_netport_lock)),
 			struct sel_netport, list);
 		list_del_rcu(&tail->list);
 		kfree_rcu(tail, rcu);

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

* [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
@ 2011-10-03 13:58 ` David Howells
  0 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2011-10-03 13:58 UTC (permalink / raw)
  To: selinux; +Cc: netdev, dhowells

Fix the following bug in sel_netport_insert() where rcu_dereference() should
be rcu_dereference_protected() as sel_netport_lock is held.

===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
security/selinux/netport.c:127 invoked rcu_dereference_check() without protection!

other info that might help us debug this:


rcu_scheduler_active = 1, debug_locks = 0
1 lock held by ossec-rootcheck/3323:
 #0:  (sel_netport_lock){+.....}, at: [<ffffffff8117d775>] sel_netport_sid+0xbb/0x226

stack backtrace:
Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
Call Trace:
 [<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
 [<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
 [<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
 [<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
 [<ffffffff810a5388>] ? might_fault+0x4e/0x9e
 [<ffffffff810a53d1>] ? might_fault+0x97/0x9e
 [<ffffffff81171cf4>] security_socket_bind+0x11/0x13
 [<ffffffff812ba967>] sys_bind+0x56/0x95
 [<ffffffff81380dac>] ? sysret_check+0x27/0x62
 [<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
 [<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
 [<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
 [<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b

Signed-off-by: David Howells <dhowells@redhat.com>
---

 security/selinux/netport.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/security/selinux/netport.c b/security/selinux/netport.c
index 0b62bd1..39e2138 100644
--- a/security/selinux/netport.c
+++ b/security/selinux/netport.c
@@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
 	if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
 		struct sel_netport *tail;
 		tail = list_entry(
-			rcu_dereference(sel_netport_hash[idx].list.prev),
+			rcu_dereference_protected(
+				sel_netport_hash[idx].list.prev,
+				spin_is_locked(&sel_netport_lock)),
 			struct sel_netport, list);
 		list_del_rcu(&tail->list);
 		kfree_rcu(tail, rcu);


--
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] 18+ messages in thread

end of thread, other threads:[~2011-12-16 13:14 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-13 14:49 [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert() David Howells
2011-12-13 23:11 ` James Morris
2011-12-16 13:13 ` David Howells
  -- strict thread matches above, loose matches on Subject: below --
2011-10-03 13:58 David Howells
2011-10-03 13:58 ` David Howells
2011-10-03 21:30 ` Paul Moore
2011-10-03 21:30   ` Paul Moore
2011-10-04  4:22   ` Eric Dumazet
2011-10-05 11:07   ` David Howells
2011-10-05 11:07     ` David Howells
2011-10-03 23:07 ` David Howells
2011-10-03 23:07   ` David Howells
2011-10-04  0:06   ` Paul Moore
2011-10-04  0:06     ` Paul Moore
2011-10-05 13:32 ` David Howells
2011-10-05 13:32   ` David Howells
2011-10-06 22:51   ` Paul Moore
2011-10-06 22:51     ` Paul Moore

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.