lkml.org 
[lkml]   [2019]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
Date
From: Eric Biggers <ebiggers@google.com>

Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
closed concurrently with fchownat(). However, it ignored that many
other proto_ops::release() methods don't set sock->sk to NULL and
therefore allow the same use-after-free:

- base_sock_release
- bnep_sock_release
- cmtp_sock_release
- data_sock_release
- dn_release
- hci_sock_release
- hidp_sock_release
- iucv_sock_release
- l2cap_sock_release
- llcp_sock_release
- llc_ui_release
- rawsock_release
- rfcomm_sock_release
- sco_sock_release
- svc_release
- vcc_release
- x25_release

Rather than fixing all these and relying on every socket type to get
this right forever, just make __sock_release() set sock->sk to NULL
itself after calling proto_ops::release().

Reproducer that produces the KASAN splat when any of these socket types
are configured into the kernel:

#include <pthread.h>
#include <stdlib.h>
#include <sys/socket.h>
#include <unistd.h>

pthread_t t;
volatile int fd;

void *close_thread(void *arg)
{
for (;;) {
usleep(rand() % 100);
close(fd);
}
}

int main()
{
pthread_create(&t, NULL, close_thread, NULL);
for (;;) {
fd = socket(rand() % 50, rand() % 11, 0);
fchownat(fd, "", 1000, 1000, 0x1000);
close(fd);
}
}

Fixes: 86741ec25462 ("net: core: Add a UID field to struct sock.")
Cc: <stable@vger.kernel.org> # v4.10+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---

NOTE: I am not an expert in the networking code, so please carefully
check that I haven't missed some reason why this simple fix won't do.

net/socket.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/socket.c b/net/socket.c
index d80d87a395ea..320f51b22b19 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -577,6 +577,7 @@ static void __sock_release(struct socket *sock, struct inode *inode)
if (inode)
inode_lock(inode);
sock->ops->release(sock);
+ sock->sk = NULL;
if (inode)
inode_unlock(inode);
sock->ops = NULL;
--
2.21.0.rc0.258.g878e2cd30e-goog
\
 
 \ /
  Last update: 2019-02-21 23:15    [W:0.068 / U:0.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site