Skip to contentSkip to conversation

virtio/vsock: Fix accept_queue memory leak

PATCH In review Tomas Krcka 2 participants 2 messages Dec 13, 2024 Stable branch

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 virtio/vsock: Fix accept_queue memory leak

From: Michal Luczaj <[email protected]>

[ Upstream commit d7b0ff5a866724c3ad21f2628c22a63336deec3f ]

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")

[Adapted due to missing commit
71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff")]

---
 net/vmw_vsock/virtio_transport_common.c | 8 ++++++++
 1 file changed, 8 insertions(+)
net/vmw_vsock/virtio_transport_common.c +8 -0
index b626c7e8e61a..ccbee1723b07 100644
@@ -1062,6 +1062,14 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt,
 		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, pkt);
+		return -ESHUTDOWN;
+	}
+
 	child = vsock_create_connected(sk);
 	if (!child) {
 		virtio_transport_reset_no_sock(t, pkt);
Signature · 1 line
2.40.1
maintainer
#1 lore raw

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: d7b0ff5a866724c3ad21f2628c22a63336deec3f

WARNING: Author mismatch between patch and upstream commit:
Backport author: Tomas Krcka <[email protected]> Commit author: Michal Luczaj <[email protected]>

Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: 897617a413e0) 6.1.y | Present (different SHA1: 946c7600fa22) 5.15.y | Not found

Note: The patch differs from the upstream commit:
---

1:  d7b0ff5a86672 < -:  ------------- virtio/vsock: Fix accept_queue memory leak
-:  ------------- > 1:  1ced3c5ad58cf virtio/vsock: Fix accept_queue memory leak

---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |
| stable/linux-5.10.y       |  Success    |  Success   |