On Tue, May 19, 2026 at 10:41 AM Stefano Garzarella <[email protected]> wrote: > > 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 Paolo, Sashiko. We'll fix the sk_ack_backlog handling. > > Thanks, > Stefano >
smime.p7s
Description: S/MIME Cryptographic Signature
