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

[Bryan Tan]

On Tue, May 19, 2026 at 11:23 AM Minh Nguyen
<minhnguyen.080505@xxxxxxxxx> wrote:
>
> vmci_transport_recv_connecting_server() returned err = 0 for a peer
> RST in its default switch arm:
>
>         err = pkt->type == VMCI_TRANSPORT_PACKET_TYPE_RST ? 0 : -EINVAL;
>
> That made vmci_transport_recv_listen() skip vsock_remove_pending(),
> leaving the pending socket on the listener's pending_links with
> sk_state = TCP_CLOSE while destroy: still dropped the explicit
> reference taken before schedule_delayed_work().
>
> One second later vsock_pending_work() observed is_pending=true and
> performed full cleanup: vsock_remove_pending() then the two trailing
> sock_put(sk) calls -- the first reached refcount 0 and __sk_freed
> the socket, and the second wrote into the freed object:
>
>   BUG: KASAN: slab-use-after-free in refcount_warn_saturate
>   Write of size 4 at addr ffff88800b1cac80 by task kworker
>   Workqueue: events vsock_pending_work
>
> Treat peer RST like any other unexpected packet type (err = -EINVAL).
> All destroy: arms now return err < 0, so vmci_transport_recv_listen()
> removes pending from pending_links synchronously and
> vsock_pending_work() takes the is_pending=false / !rejected branch,
> dropping only its own work reference.  This also closes the
> multi-packet race Sashiko reported on v2: pending is removed from
> the list before any subsequent packet can find it.
>
> The pre-existing sk_acceptq_removed() gap on the err < 0 path of
> vmci_transport_recv_listen() that Sashiko also noted is not
> introduced or changed by this patch.
>
> Tested on lts-6.12.79 with KASAN: 52/100 unpatched -> 0/100 patched.
>
> Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Minh Nguyen <minhnguyen.080505@xxxxxxxxx>
> Assisted-by: Claude:claude-opus-4-7
> ---
> v4:
>   - Resend as an independent thread per netdev workflow (v3 was
>     incorrectly posted in-reply-to the v2 thread).
>   - Drop the inline comment expansion; keep the original
>     /* Close and cleanup the connection. */.  No functional change.
>
> v3:
>   - Different approach to Sashiko/Paolo's "trading UAF for leak"
>     concern: normalize RST to err = -EINVAL so all destroy: arms
>     take the same err < 0 cleanup path -- no special case, no
>     multi-packet race.
>   - Sashiko's secondary observation ("while not introduced by this
>     patch, does this error path leak sk_ack_backlog slots on failed
>     handshakes?") is correct: the sk_acceptq_removed() gap on the
>     err < 0 branch of vmci_transport_recv_listen() is pre-existing
>     and is not introduced or changed by this patch.  A separate fix
>     for that gap is needed and would be welcome.
>
> v2: https://lore.kernel.org/netdev/20260512025851.189140-1-minhnguyen.080505@xxxxxxxxx/
>
> v1 was sent to security@xxxxxxxxxx on 2026-05-10 (not on lore).
>
>  net/vmw_vsock/vmci_transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Bryan Tan <bryan-bt.tan@xxxxxxxxxxxx>

>
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 4296ca1..d257938 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -1164,7 +1164,7 @@ vmci_transport_recv_connecting_server(struct sock *listener,
>                 /* Close and cleanup the connection. */
>                 vmci_transport_send_reset(pending, pkt);
>                 skerr = EPROTO;
> -               err = pkt->type == VMCI_TRANSPORT_PACKET_TYPE_RST ? 0 : -EINVAL;
> +               err = -EINVAL;
>                 goto destroy;
>         }
>
>
> base-commit: be48e5fe51a5864566307998286a699d6b986934
> --
> 2.54.0
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature