On Thu, Feb 21, 2019 at 2:14 PM Eric Biggers <[email protected]> wrote: > > From: Eric Biggers <[email protected]> > > 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: <[email protected]> # v4.10+ > Signed-off-by: Eric Biggers <[email protected]>
Acked-by: Cong Wang <[email protected]>
