| Message ID | 20241106-vsock-mem-leaks-v1-1-8f4ffc3099e6@rbox.co (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | virtio/vsock: Fix memory leaks | expand |
On Wed, Nov 06, 2024 at 06:51:18PM +0100, Michal Luczaj wrote: >As the final stages of socket destruction may be delayed, it is possible >that virtio_transport_recv_listen() will be called after the accept_queue >has been flushed, but before the SOCK_DONE flag has been set. As a result, >sockets enqueued after the flush would remain unremoved, leading to a >memory leak. > >vsock_release > __vsock_release > lock > virtio_transport_release > virtio_transport_close > schedule_delayed_work(close_work) > sk_shutdown = SHUTDOWN_MASK >(!) flush accept_queue > release > virtio_transport_recv_pkt > vsock_find_bound_socket > lock > if flag(SOCK_DONE) return > virtio_transport_recv_listen > child = vsock_create_connected > (!) vsock_enqueue_accept(child) > release >close_work > lock > virtio_transport_do_close > set_flag(SOCK_DONE) > virtio_transport_remove_sock > vsock_remove_sock > vsock_remove_bound > release > >Introduce a sk_shutdown check to disallow vsock_enqueue_accept() during >socket destruction. > >unreferenced object 0xffff888109e3f800 (size 2040): > comm "kworker/5:2", pid 371, jiffies 4294940105 > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 28 00 0b 40 00 00 00 00 00 00 00 00 00 00 00 00 (..@............ > backtrace (crc 9e5f4e84): > [<ffffffff81418ff1>] kmem_cache_alloc_noprof+0x2c1/0x360 > [<ffffffff81d27aa0>] sk_prot_alloc+0x30/0x120 > [<ffffffff81d2b54c>] sk_alloc+0x2c/0x4b0 > [<ffffffff81fe049a>] __vsock_create.constprop.0+0x2a/0x310 > [<ffffffff81fe6d6c>] virtio_transport_recv_pkt+0x4dc/0x9a0 > [<ffffffff81fe745d>] vsock_loopback_work+0xfd/0x140 > [<ffffffff810fc6ac>] process_one_work+0x20c/0x570 > [<ffffffff810fce3f>] worker_thread+0x1bf/0x3a0 > [<ffffffff811070dd>] kthread+0xdd/0x110 > [<ffffffff81044fdd>] ret_from_fork+0x2d/0x50 > [<ffffffff8100785a>] ret_from_fork_asm+0x1a/0x30 > >Fixes: 3fe356d58efa ("vsock/virtio: discard packets only when socket is really closed") >Signed-off-by: Michal Luczaj <mhal@rbox.co> >--- > net/vmw_vsock/virtio_transport_common.c | 8 ++++++++ > 1 file changed, 8 insertions(+) Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >index ccbd2bc0d2109aea4f19e79a0438f85893e1d89c..cd075f608d4f6f48f894543e5e9c966d3e5f22df 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -1512,6 +1512,14 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb, > return -ENOMEM; > } > >+ /* __vsock_release() might have already flushed accept_queue. >+ * Subsequent enqueues would lead to a memory leak. >+ */ >+ if (sk->sk_shutdown == SHUTDOWN_MASK) { >+ virtio_transport_reset_no_sock(t, skb); >+ return -ESHUTDOWN; >+ } >+ > child = vsock_create_connected(sk); > if (!child) { > virtio_transport_reset_no_sock(t, skb); > >-- >2.46.2 >
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index ccbd2bc0d2109aea4f19e79a0438f85893e1d89c..cd075f608d4f6f48f894543e5e9c966d3e5f22df 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -1512,6 +1512,14 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb, return -ENOMEM; } + /* __vsock_release() might have already flushed accept_queue. + * Subsequent enqueues would lead to a memory leak. + */ + if (sk->sk_shutdown == SHUTDOWN_MASK) { + virtio_transport_reset_no_sock(t, skb); + return -ESHUTDOWN; + } + child = vsock_create_connected(sk); if (!child) { virtio_transport_reset_no_sock(t, skb);
As the final stages of socket destruction may be delayed, it is possible that virtio_transport_recv_listen() will be called after the accept_queue has been flushed, but before the SOCK_DONE flag has been set. As a result, sockets enqueued after the flush would remain unremoved, leading to a memory leak. vsock_release __vsock_release lock virtio_transport_release virtio_transport_close schedule_delayed_work(close_work) sk_shutdown = SHUTDOWN_MASK (!) flush accept_queue release virtio_transport_recv_pkt vsock_find_bound_socket lock if flag(SOCK_DONE) return virtio_transport_recv_listen child = vsock_create_connected (!) vsock_enqueue_accept(child) release close_work lock virtio_transport_do_close set_flag(SOCK_DONE) virtio_transport_remove_sock vsock_remove_sock vsock_remove_bound release Introduce a sk_shutdown check to disallow vsock_enqueue_accept() during socket destruction. unreferenced object 0xffff888109e3f800 (size 2040): comm "kworker/5:2", pid 371, jiffies 4294940105 hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 28 00 0b 40 00 00 00 00 00 00 00 00 00 00 00 00 (..@............ backtrace (crc 9e5f4e84): [<ffffffff81418ff1>] kmem_cache_alloc_noprof+0x2c1/0x360 [<ffffffff81d27aa0>] sk_prot_alloc+0x30/0x120 [<ffffffff81d2b54c>] sk_alloc+0x2c/0x4b0 [<ffffffff81fe049a>] __vsock_create.constprop.0+0x2a/0x310 [<ffffffff81fe6d6c>] virtio_transport_recv_pkt+0x4dc/0x9a0 [<ffffffff81fe745d>] vsock_loopback_work+0xfd/0x140 [<ffffffff810fc6ac>] process_one_work+0x20c/0x570 [<ffffffff810fce3f>] worker_thread+0x1bf/0x3a0 [<ffffffff811070dd>] kthread+0xdd/0x110 [<ffffffff81044fdd>] ret_from_fork+0x2d/0x50 [<ffffffff8100785a>] ret_from_fork_asm+0x1a/0x30 Fixes: 3fe356d58efa ("vsock/virtio: discard packets only when socket is really closed") Signed-off-by: Michal Luczaj <mhal@rbox.co> --- net/vmw_vsock/virtio_transport_common.c | 8 ++++++++ 1 file changed, 8 insertions(+)