Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH net] vsock/virtio: validate packet source for connected sockets
@ 2026-08-13 12:12 Daehyeon Ko
  2026-08-14 12:23 ` Stefano Garzarella
  2026-08-17 17:56 ` Bobby Eshleman
  0 siblings, 2 replies; 3+ messages in thread
From: Daehyeon Ko @ 2026-08-13 12:12 UTC (permalink / raw)
  To: netdev
  Cc: Stefan Hajnoczi, Stefano Garzarella, virtualization, kvm,
	Daehyeon Ko

virtio_transport_recv_pkt() first looks up a socket using the full source
and destination tuple.  If that misses, it falls back to a bound-socket
lookup using only the destination address.  The fallback is needed for
listening and connecting sockets, but it can also select an established
socket that remains in the bound table.

As a result, a packet from an unrelated source can be dispatched to a
non-listening socket.  In TCP_SYN_SENT, a source-blind RESPONSE marks the
selected socket established while retaining its original remote address.
Subsequent RW packets can likewise be delivered through the
destination-only fallback.

This was reproduced with two capless processes under different UIDs.  The
attacker discovered the victim tuple through unprivileged AF_VSOCK
sock_diag and injected a chosen 16-byte payload into the victim established
loopback socket.  The legitimate peer received none of those bytes.

After taking the socket lock, verify that packets for non-listening sockets
come from the peer stored in remote_addr.  Listening sockets continue to
accept packets from any source.

Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
Cc: stable@vger.kernel.org
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
 net/vmw_vsock/virtio_transport_common.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 8becad812..f73e0406a 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1822,11 +1822,15 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
 
 	lock_sock(sk);
 
-	/* Check if sk has been closed or assigned to another transport before
-	 * lock_sock (note: listener sockets are not assigned to any transport)
+	/* Check if sk has been closed, assigned to another transport, or if the
+	 * packet is from a different peer than the one connected to sk.  These
+	 * properties could have changed before lock_sock.  Listener sockets are
+	 * not assigned to any transport and accept packets from any peer.
 	 */
 	if (sock_flag(sk, SOCK_DONE) ||
-	    (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
+	    (sk->sk_state != TCP_LISTEN &&
+	     (vsk->transport != &t->transport ||
+	      !vsock_addr_equals_addr(&src, &vsk->remote_addr)))) {
 		(void)virtio_transport_reset_no_sock(t, skb, net);
 		release_sock(sk);
 		sock_put(sk);

base-commit: 9006c116dd111d457bf5d074990210f70a4ad2c8
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] vsock/virtio: validate packet source for connected sockets
  2026-08-13 12:12 [PATCH net] vsock/virtio: validate packet source for connected sockets Daehyeon Ko
@ 2026-08-14 12:23 ` Stefano Garzarella
  2026-08-17 17:56 ` Bobby Eshleman
  1 sibling, 0 replies; 3+ messages in thread
From: Stefano Garzarella @ 2026-08-14 12:23 UTC (permalink / raw)
  To: Daehyeon Ko; +Cc: netdev, Stefan Hajnoczi, virtualization, kvm

On Thu, Aug 13, 2026 at 09:12:36PM +0900, Daehyeon Ko wrote:
>virtio_transport_recv_pkt() first looks up a socket using the full source
>and destination tuple.  If that misses, it falls back to a bound-socket
>lookup using only the destination address.  The fallback is needed for
>listening and connecting sockets, but it can also select an established
>socket that remains in the bound table.
>
>As a result, a packet from an unrelated source can be dispatched to a
>non-listening socket.  In TCP_SYN_SENT, a source-blind RESPONSE marks the
>selected socket established while retaining its original remote address.
>Subsequent RW packets can likewise be delivered through the
>destination-only fallback.
>
>This was reproduced with two capless processes under different UIDs.  The
>attacker discovered the victim tuple through unprivileged AF_VSOCK
>sock_diag and injected a chosen 16-byte payload into the victim established
>loopback socket.  The legitimate peer received none of those bytes.

I don't understand this, what it means? If the receiver doesn't receive
those injected bytes, should be fine, no?

>
>After taking the socket lock, verify that packets for non-listening sockets
>come from the peer stored in remote_addr.  Listening sockets continue to
>accept packets from any source.

IMO this description should be improved, it's quite hard to follow.
This is an idea IIUC the issue:

   virtio_transport_recv_pkt() looks up sockets in two steps: first by
   the full source and destination tuple in the connected table, then by
   destination only in the bound table. The fallback is needed for
   listening and connecting sockets, but it never validates the source
   address in the packet header. Since sockets remain in the bound table
   after connect(), a packet from any source can be dispatched to a
   non-listening socket.

   An unprivileged process can discover the target tuple through AF_VSOCK
   sock_diag (no capabilities required), send a forged RESPONSE to a
   TCP_SYN_SENT socket to establish the connection, and then inject
   arbitrary data through subsequent RW packets, all delivered via the
   destination-only fallback.

   After lock_sock(), verify that the packet source matches the
   remote_addr stored during connect().

>
>Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
>Cc: stable@vger.kernel.org
>Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 8becad812..f73e0406a 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1822,11 +1822,15 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
>
> 	lock_sock(sk);
>
>-	/* Check if sk has been closed or assigned to another transport before
>-	 * lock_sock (note: listener sockets are not assigned to any transport)
>+	/* Check if sk has been closed, assigned to another transport, or if the
>+	 * packet is from a different peer than the one connected to sk.  These
>+	 * properties could have changed before lock_sock.  Listener sockets are
>+	 * not assigned to any transport and accept packets from any peer.

Please keeps changes minimal, why adding "These properties could have
changed before lock_sock." and changing the phrase related to listener
socket?

Also the new part is not clear IMO, we should explain why we are adding
this new check, not what we are doing which is clear looking at the
code.

IMO we should just add something like this (feel free to change it):

   Also verify the packet source: the bound-table fallback lookup
   matches by destination only, so a non-listening socket can be
   reached by a packet from an unrelated peer. Drop it unless the
   source matches the peer the socket is connecting or connected to.

> 	 */
> 	if (sock_flag(sk, SOCK_DONE) ||
>-	    (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
>+	    (sk->sk_state != TCP_LISTEN &&
>+	     (vsk->transport != &t->transport ||
>+	      !vsock_addr_equals_addr(&src, &vsk->remote_addr)))) {

Do we need to add the same check also in the VMCI transport?

Hyper-V seems different.

Thanks,
Stefano

> 		(void)virtio_transport_reset_no_sock(t, skb, net);
> 		release_sock(sk);
> 		sock_put(sk);
>
>base-commit: 9006c116dd111d457bf5d074990210f70a4ad2c8
>-- 
>2.54.0
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] vsock/virtio: validate packet source for connected sockets
  2026-08-13 12:12 [PATCH net] vsock/virtio: validate packet source for connected sockets Daehyeon Ko
  2026-08-14 12:23 ` Stefano Garzarella
@ 2026-08-17 17:56 ` Bobby Eshleman
  1 sibling, 0 replies; 3+ messages in thread
From: Bobby Eshleman @ 2026-08-17 17:56 UTC (permalink / raw)
  To: Daehyeon Ko
  Cc: netdev, Stefan Hajnoczi, Stefano Garzarella, virtualization, kvm

On Thu, Aug 13, 2026 at 09:12:36PM +0900, Daehyeon Ko wrote:
> virtio_transport_recv_pkt() first looks up a socket using the full source
> and destination tuple.  If that misses, it falls back to a bound-socket
> lookup using only the destination address.  The fallback is needed for
> listening and connecting sockets, but it can also select an established
> socket that remains in the bound table.
> 
> As a result, a packet from an unrelated source can be dispatched to a
> non-listening socket.  In TCP_SYN_SENT, a source-blind RESPONSE marks the
> selected socket established while retaining its original remote address.
> Subsequent RW packets can likewise be delivered through the
> destination-only fallback.
> 
> This was reproduced with two capless processes under different UIDs.  The
> attacker discovered the victim tuple through unprivileged AF_VSOCK
> sock_diag and injected a chosen 16-byte payload into the victim established
> loopback socket.  The legitimate peer received none of those bytes.
> 
> After taking the socket lock, verify that packets for non-listening sockets
> come from the peer stored in remote_addr.  Listening sockets continue to
> accept packets from any source.
> 
> Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 8becad812..f73e0406a 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -1822,11 +1822,15 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
>  
>  	lock_sock(sk);
>  
> -	/* Check if sk has been closed or assigned to another transport before
> -	 * lock_sock (note: listener sockets are not assigned to any transport)
> +	/* Check if sk has been closed, assigned to another transport, or if the
> +	 * packet is from a different peer than the one connected to sk.  These
> +	 * properties could have changed before lock_sock.  Listener sockets are
> +	 * not assigned to any transport and accept packets from any peer.
>  	 */
>  	if (sock_flag(sk, SOCK_DONE) ||
> -	    (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
> +	    (sk->sk_state != TCP_LISTEN &&
> +	     (vsk->transport != &t->transport ||
> +	      !vsock_addr_equals_addr(&src, &vsk->remote_addr)))) {

Does this equality work for loopback, when the CID may be
VMADDR_CID_LOCAL and/or VMADDR_CID_HOST on host or the guest CID in
guest?

Best,
Bobby

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-17 17:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 12:12 [PATCH net] vsock/virtio: validate packet source for connected sockets Daehyeon Ko
2026-08-14 12:23 ` Stefano Garzarella
2026-08-17 17:56 ` Bobby Eshleman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox