Re: [PATCH net v2] vsock/vmci: fix UAF when peer resets connection during handshake

[Stefano Garzarella <sgarzare@redhat.com>]

On Thu, May 14, 2026 at 03:26:28PM +0200, Paolo Abeni wrote:
>On 5/12/26 4:58 AM, Minh Nguyen wrote:
>> vmci_transport_recv_connecting_server() jumps to its destroy: label
>> and performs an unconditional sock_put(pending) to release the
>> explicit sock_hold() taken by vmci_transport_recv_listen() before
>> schedule_delayed_work().  The existing comment claimed this was safe
>> because the listen handler removes pending from the pending list on
>> the way out, which would prevent vsock_pending_work() from dropping
>> the same reference later.
>

[...]

>Sashiko says:
>
>---
>Could this change lead to a socket memory leak if another packet arrives
>before vsock_pending_work() executes?
>If a peer RST is received (err == 0), the socket stays on the
>pending_links list with its state set to TCP_CLOSE, and the base
>reference is kept.
>If the peer then sends another packet (such as another RST) within the
>delay window before vsock_pending_work() runs,
>vmci_transport_get_pending() might find this same socket.
>Since its state is TCP_CLOSE, vmci_transport_recv_listen() would hit the
>default switch case, set err = -EINVAL, and call vsock_remove_pending().
>This removes the socket from the list and drops the list reference, but
>it bypasses vmci_transport_recv_connecting_server(), meaning the base
>reference is never dropped.
>When vsock_pending_work() runs later, vsock_is_pending() evaluates to false.
>This sets cleanup = false and bypasses the sock_put(sk) call, leaking
>the pending socket.
>While not introduced by this patch, does this error path leak
>sk_ack_backlog slots on failed handshakes?
>If a handshake fails due to an error, vmci_transport_recv_listen()
>handles it by calling vsock_remove_pending(). This removes the socket
>from the pending_links list but does not call sk_acceptq_removed(sk).
>When vsock_pending_work() runs later, vsock_is_pending() evaluates to
>false because the socket is no longer in the list. This causes the work
>function to skip its own sk_acceptq_removed(listener) call, meaning the
>listener's sk_ack_backlog is never decremented.
>---
>
>it looks like the above is trading an UaF for a leak ?!?
>

@Minh @Bryan can you check this report?
It seems a real issue, so the patch was not applied.

Thanks,
Stefano