netdev - [patch] selinux: fix selinux_netlbl_inode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 2 Jan 2007 21:09:31 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Andrew Morton <akpm@...l.org>
Cc:	"Adam J. Richter" <adam@...drasil.com>,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	Paul Moore <paul.moore@...com>,
	Linus Torvalds <torvalds@...l.org>
Subject: [patch] selinux: fix selinux_netlbl_inode_permission() locking

* Andrew Morton <akpm@...l.org> wrote:

> There's a glaring bug in selinux_netlbl_inode_permission() - taking 
> lock_sock() inside rcu_read_lock().

Note that the bug is still in -rc3, and is easily triggerable via a 
default FC6 bootup. It's fixed by the (slightly modified) patch from 
Parag Warudkar below that i have in the -rt tree.

Note that this bug became visible due to the recent __resched_legal() 
fix, which bug made most of our atomicity debugging checks ineffective. 
About half a dozen separate atomicity bugs triggered in -rt when i fixed 
the __resched_legal() bug, so i'd expect some more to trigger upstream 
too.

	Ingo

------------------------>
Subject: [patch] selinux: fix selinux_netlbl_inode_permission() locking
From: Parag Warudkar <paragw@...agw.zapto.org>

do not call a sleeping lock API in an RCU read section.
lock_sock_nested can sleep, its BH counterpart doesn't. 
selinux_netlbl_inode_permission() needs to use the BH counterpart
unconditionally.

Compile tested.

From: Ingo Molnar <mingo@...e.hu>

added BH disabling, because this function can be called from non-atomic
contexts too, so a naked bh_lock_sock() would be deadlock-prone.

Boot-tested the resulting kernel.

Signed-off-by: Parag Warudkar <paragw@...agw.zapto.org>
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
 security/selinux/ss/services.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux/security/selinux/ss/services.c
===================================================================
--- linux.orig/security/selinux/ss/services.c
+++ linux/security/selinux/ss/services.c
@@ -2660,9 +2660,11 @@ int selinux_netlbl_inode_permission(stru
 		rcu_read_unlock();
 		return 0;
 	}
-	lock_sock(sock->sk);
+	local_bh_disable();
+	bh_lock_sock_nested(sock->sk);
 	rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
-	release_sock(sock->sk);
+	bh_unlock_sock(sock->sk);
+	local_bh_enable();
 	rcu_read_unlock();

 	return rc;
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html