Thread (30 messages) flat view 30 messages, 6 authors, 2011-03-22

Re: [held lock freed] Re: [GIT] Networking

From: Ingo Molnar <hidden>
Date: 2011-03-21 16:16:34
Also in: lkml

* Arnd Bergmann [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Monday 21 March 2011, Eric Dumazet wrote:
quoted
quoted
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 4680b1e..b2cf1db 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -669,8 +669,8 @@ static int x25_release(struct socket *sock)
 
      sock_orphan(sk);
 out:
-     release_sock(sk);
      sock_put(sk);
+     release_sock(sk);
      return 0;
 }
 
Hmm, x25_release() looks fine, I believe no fix is needed.
D'oh. You're right of course. My patch would introduce the bug,
not fix it. Updated patch below.

	Arnd
8<-------------------------
net/appletalk: fix atalk_release use after free

The BKL removal in appletalk introduced a use-after-free problem,
where atalk_destroy_socket frees a sock, but we still release
the socket lock on it.

An easy fix is to take an extra reference on the sock and sock_put
it when returning from atalk_release.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

index 3d4f4b0..206e771 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1051,6 +1051,7 @@ static int atalk_release(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
 
+	sock_hold(sk);
 	lock_sock(sk);
 	if (sk) {
 		sock_orphan(sk);
@@ -1058,6 +1059,8 @@ static int atalk_release(struct socket *sock)
 		atalk_destroy_socket(sk);
 	}
 	release_sock(sk);
+	sock_put(sk);
+
 	return 0;
 }
I have not triggered the lockdep warning in this code yet - but i've ran the 
patch and have seen no ill effects from it so far.

Thanks,

	Ingo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help