Re: NULL pointer deref, selinux_socket_unix_may_send+0x34/0x90

[Paul Moore <paul@paul-moore.com>]

[-- Attachment #1: Type: text/plain, Size: 2995 bytes --]

On Thursday, March 21, 2013 11:19:22 PM Ján Stanček wrote:
> Hi,
> 
> I'm occasionally seeing a panic early after system booted and while
> systemd is starting other services.
> 
> I made a reproducer which is quite reliable on my system (32 CPU Intel)
> and can usually trigger this issue within a minute or two. I can reproduce
> this issue with 3.9.0-rc3 as root or unprivileged user (see call trace
> below).
> 
> I'm attaching my reproducer and (experimental) patch, which fixes the
> issue for me.

Hi Jan,

I've heard some similar reports over the past few years but I've never been 
able to reproduce the problem and the reporters have never show enough 
interest to be able to help me diagnose the problem.  Your information about 
the size of the machine and the reproducer may help, thank you!

I'll try your reproducer but since I don't happen to have a machine handy that 
is the same size as yours would you mind trying the attached (also pasted 
inline for others to comment on) patch?  I can't promise it will solve your 
problem but it was the best idea I could come up with a few years ago when I 
first became aware of the problem.  I think you are right in that there is a 
race condition somewhere with the AF_UNIX sockets shutting down, I'm just not 
yet certain where it is ...

Thanks again.

net: fix some potential race issues in the AF_UNIX code

From: Paul Moore <pmoore@redhat.com>

At least one user had reported some odd behavior with UNIX sockets which
could be attributed to some _possible_ race conditions in
unix_release_sock().

Reported-by: Konstantin Boyandin <konstantin@boyandin.com>
Reported-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
 net/unix/af_unix.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 51be64f..886b8da 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -408,8 +408,10 @@ static int unix_release_sock(struct sock *sk, int 
embrion)
 	skpair = unix_peer(sk);
 
 	if (skpair != NULL) {
-		if (sk->sk_type == SOCK_STREAM || sk->sk_type == SOCK_SEQPACKET) {
-			unix_state_lock(skpair);
+		unix_state_lock(skpair);
+		if (unix_our_peer(sk, skpair) &&
+		    (sk->sk_type == SOCK_STREAM ||
+		     sk->sk_type == SOCK_SEQPACKET)) {
 			/* No more writes */
 			skpair->sk_shutdown = SHUTDOWN_MASK;
 			if (!skb_queue_empty(&sk->sk_receive_queue) || embrion)
@@ -417,9 +419,10 @@ static int unix_release_sock(struct sock *sk, int 
embrion)
 			unix_state_unlock(skpair);
 			skpair->sk_state_change(skpair);
 			sk_wake_async(skpair, SOCK_WAKE_WAITD, POLL_HUP);
-		}
-		sock_put(skpair); /* It may now die */
+		} else
+			unix_state_unlock(skpair);
 		unix_peer(sk) = NULL;
+		sock_put(skpair); /* It may now die */
 	}
 
 	/* Try to flush out this socket. Throw out buffers at least */


-- 
paul moore
www.paul-moore.com

[-- Attachment #2: unix-race_fix.patch --]
[-- Type: text/x-patch, Size: 1555 bytes --]

net: fix some potential race issues in the AF_UNIX code

From: Paul Moore <pmoore@redhat.com>

At least one user had reported some odd behavior with UNIX sockets which
could be attributed to some _possible_ race conditions in
unix_release_sock().

Reported-by: Konstantin Boyandin <konstantin@boyandin.com>
Reported-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
 net/unix/af_unix.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 51be64f..886b8da 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -408,8 +408,10 @@ static int unix_release_sock(struct sock *sk, int embrion)
 	skpair = unix_peer(sk);
 
 	if (skpair != NULL) {
-		if (sk->sk_type == SOCK_STREAM || sk->sk_type == SOCK_SEQPACKET) {
-			unix_state_lock(skpair);
+		unix_state_lock(skpair);
+		if (unix_our_peer(sk, skpair) &&
+		    (sk->sk_type == SOCK_STREAM ||
+		     sk->sk_type == SOCK_SEQPACKET)) {
 			/* No more writes */
 			skpair->sk_shutdown = SHUTDOWN_MASK;
 			if (!skb_queue_empty(&sk->sk_receive_queue) || embrion)
@@ -417,9 +419,10 @@ static int unix_release_sock(struct sock *sk, int embrion)
 			unix_state_unlock(skpair);
 			skpair->sk_state_change(skpair);
 			sk_wake_async(skpair, SOCK_WAKE_WAITD, POLL_HUP);
-		}
-		sock_put(skpair); /* It may now die */
+		} else
+			unix_state_unlock(skpair);
 		unix_peer(sk) = NULL;
+		sock_put(skpair); /* It may now die */
 	}
 
 	/* Try to flush out this socket. Throw out buffers at least */