netdev - Re: appletalk oops.

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20110331.185823.232755679.davem@davemloft.net>
Date:	Thu, 31 Mar 2011 18:58:23 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	davej@...hat.com
Cc:	netdev@...r.kernel.org, arnd@...db.de
Subject: Re: appletalk oops.

From: Dave Jones <davej@...hat.com>
Date: Thu, 31 Mar 2011 16:05:26 -0400

> Just hit this on current git head while fuzzing syscalls.
> I suspect we need to check somewhere for null sock's being passed in from userspace
> I'm not sure yet this is appletalk specific, or it belongs somewhere further up
> in accept.

Turns out atalk_release() is completely awesome after the
lock_kernel() conversion.

It grabs a reference to a socket, then checks if that socket is NULL,
right afterwards!

And this NULL socket case is exactly what happens if you try to do an
accept() on an Appletalk socket, since it hooks up sock_no_accept().

This is the second regression in this function due to commit
60d9f461a20ba59219fdcdc30cbf8e3a4ad3f625 ("appletalk: remove the
BKL"):

--------------------
appletalk: Fix OOPS in atalk_release().

Commit 60d9f461a20ba59219fdcdc30cbf8e3a4ad3f625 ("appletalk: remove
the BKL") added a dereference of "sk" before checking for NULL in
atalk_release().

Guard the code block completely, rather than partially, with the
NULL check.

Reported-by: Dave Jones <davej@...hat.com>
Signed-off-by: David S. Miller <davem@...emloft.net>

diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 206e771..956a530 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1051,16 +1051,17 @@ static int atalk_release(struct socket *sock)
 {
 	struct sock *sk = sock->sk;

-	sock_hold(sk);
-	lock_sock(sk);
 	if (sk) {
+		sock_hold(sk);
+		lock_sock(sk);
+
 		sock_orphan(sk);
 		sock->sk = NULL;
 		atalk_destroy_socket(sk);
-	}
-	release_sock(sk);
-	sock_put(sk);

+		release_sock(sk);
+		sock_put(sk);
+	}
 	return 0;
 }

--
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