net: socket: set sock->sk to NULL after calling proto_ops::release() [Linux 5.0]

net: socket: set sock->sk to NULL after calling proto_ops::release() [Linux 5.0]

This Linux kernel change "net: socket: set sock->sk to NULL after calling proto_ops::release()" is included in the Linux 5.0 release. This change is authored by Eric Biggers <ebiggers [at] google.com> on Thu Feb 21 14:13:56 2019 -0800. The commit for this change in Linux stable tree is ff7b11a (patch).

net: socket: set sock->sk to NULL after calling proto_ops::release()

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.")
Signed-off-by: Eric Biggers <[email protected]>
Acked-by: Cong Wang <[email protected]>
Signed-off-by: David S. Miller <[email protected]>

There is one line of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

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

diff --git a/net/socket.c b/net/socket.c
index d80d87a..320f51b 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;

Leave a Reply

Your email address will not be published. Required fields are marked *