Skip to content

Commit b69546c

Browse files
pvts-matPlaidCat
authored andcommitted
vsock: Ignore signal/timeout on connect() if already established
jira VULN-160775 cve CVE-2025-40248 commit-author Michal Luczaj <mhal@rbox.co> commit 002541e upstream-diff The modified fragment is missing the backports: - 380feae ("vsock: cancel packets when failing to connect"), - c7ff9cf ("vsock: notify server to shutdown when client has pending signal"), - b920849 ("vsock: remove vsock from connected table when connect is interrupted by a signal"), - 6d4486e ("vsock: avoid to close connected socket after the timeout"). For the established connection case their lack is inconsequential, because the fix just drops entirely the behavior which was merely adjusted by them. For the not established connection case the `vsock_transport_cancel_pkt(vsk)' call was dropped, compared to the upstream, to honor the lack of 380feae backport. During connect(), acting on a signal/timeout by disconnecting an already established socket leads to several issues: 1. connect() invoking vsock_transport_cancel_pkt() -> virtio_transport_purge_skbs() may race with sendmsg() invoking virtio_transport_get_credit(). This results in a permanently elevated `vvs->bytes_unsent`. Which, in turn, confuses the SOCK_LINGER handling. 2. connect() resetting a connected socket's state may race with socket being placed in a sockmap. A disconnected socket remaining in a sockmap breaks sockmap's assumptions. And gives rise to WARNs. 3. connect() transitioning SS_CONNECTED -> SS_UNCONNECTED allows for a transport change/drop after TCP_ESTABLISHED. Which poses a problem for any simultaneous sendmsg() or connect() and may result in a use-after-free/null-ptr-deref. Do not disconnect socket on signal/timeout. Keep the logic for unconnected sockets: they don't linger, can't be placed in a sockmap, are rejected by sendmsg(). [1]: https://lore.kernel.org/netdev/e07fd95c-9a38-4eea-9638-133e38c2ec9b@rbox.co/ [2]: https://lore.kernel.org/netdev/20250317-vsock-trans-signal-race-v4-0-fc8837f3f1d4@rbox.co/ [3]: https://lore.kernel.org/netdev/60f1b7db-3099-4f6a-875e-af9f6ef194f6@rbox.co/ Fixes: d021c34 ("VSOCK: Introduce VM Sockets") Signed-off-by: Michal Luczaj <mhal@rbox.co> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Link: https://patch.msgid.link/20251119-vsock-interrupted-connect-v2-1-70734cf1233f@rbox.co Signed-off-by: Jakub Kicinski <kuba@kernel.org> (cherry picked from commit 002541e) Signed-off-by: Marcin Wcisło <marcin.wcislo@conclusive.pl>
1 parent c322c7b commit b69546c

1 file changed

Lines changed: 27 additions & 7 deletions

File tree

net/vmw_vsock/af_vsock.c

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,15 +1221,35 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
12211221
timeout = schedule_timeout(timeout);
12221222
lock_sock(sk);
12231223

1224-
if (signal_pending(current)) {
1225-
err = sock_intr_errno(timeout);
1226-
sk->sk_state = TCP_CLOSE;
1227-
sock->state = SS_UNCONNECTED;
1228-
goto out_wait;
1229-
} else if (timeout == 0) {
1230-
err = -ETIMEDOUT;
1224+
/* Connection established. Whatever happens to socket once we
1225+
* release it, that's not connect()'s concern. No need to go
1226+
* into signal and timeout handling. Call it a day.
1227+
*
1228+
* Note that allowing to "reset" an already established socket
1229+
* here is racy and insecure.
1230+
*/
1231+
if (sk->sk_state == TCP_ESTABLISHED)
1232+
break;
1233+
1234+
/* If connection was _not_ established and a signal/timeout came
1235+
* to be, we want the socket's state reset. User space may want
1236+
* to retry.
1237+
*
1238+
* sk_state != TCP_ESTABLISHED implies that socket is not on
1239+
* vsock_connected_table. We keep the binding and the transport
1240+
* assigned.
1241+
*/
1242+
if (signal_pending(current) || timeout == 0) {
1243+
err = timeout == 0 ? -ETIMEDOUT : sock_intr_errno(timeout);
1244+
1245+
/* Listener might have already responded with
1246+
* VIRTIO_VSOCK_OP_RESPONSE. Its handling expects our
1247+
* sk_state == TCP_SYN_SENT, which hereby we break.
1248+
* In such case VIRTIO_VSOCK_OP_RST will follow.
1249+
*/
12311250
sk->sk_state = TCP_CLOSE;
12321251
sock->state = SS_UNCONNECTED;
1252+
12331253
goto out_wait;
12341254
}
12351255

0 commit comments

Comments
 (0)