lkml.org 
[lkml]   [2019]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 023/135] net: socket: set sock->sk to NULL after calling proto_ops::release()
    Date
    4.14-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Eric Biggers <ebiggers@google.com>

    [ Upstream commit ff7b11aa481f682e0e9711abfeb7d03f5cd612bf ]

    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 <ebiggers@google.com>
    Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/socket.c | 1 +
    1 file changed, 1 insertion(+)

    --- a/net/socket.c
    +++ b/net/socket.c
    @@ -600,6 +600,7 @@ static void __sock_release(struct socket
    if (inode)
    inode_lock(inode);
    sock->ops->release(sock);
    + sock->sk = NULL;
    if (inode)
    inode_unlock(inode);
    sock->ops = NULL;

    \
     
     \ /
      Last update: 2019-03-12 18:17    [W:5.025 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site