From: Bobby Eshleman <bobbyeshleman@meta.com>

Add NS support to vsock loopback. Sockets in a global mode netns
communicate with each other, regardless of namespace. Sockets in a local
mode netns may only communicate with other sockets within the same
namespace.

Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v9:
- remove per-netns vsock_loopback and workqueues, just re-using
  the net and net_mode in skb->cb achieved the same result in a simpler
  way. Also removed need for pernet_subsys.
- properly track net references

Changes in v7:
- drop for_each_net() init/exit, drop net_rwsem, the pernet registration
  handles this automatically and race-free
- flush workqueue before destruction, purge pkt list
- remember net_mode instead of current net mode
- keep space after INIT_WORK()
- change vsock_loopback in netns_vsock to ->priv void ptr
- rename `orig_net_mode` to `net_mode`
- remove useless comment
- protect `register_pernet_subsys()` with `net_rwsem`
- do cleanup before releasing `net_rwsem` when failure happens
- call `unregister_pernet_subsys()` in `vsock_loopback_exit()`
- call `vsock_loopback_deinit_vsock()` in `vsock_loopback_exit()`

Changes in v6:
- init pernet ops for vsock_loopback module
- vsock_loopback: add space in struct to clarify lock protection
- do proper cleanup/unregister on vsock_loopback_exit()
- vsock_loopback: use virtio_vsock_skb_net()

Changes in v5:
- add callbacks code to avoid reverse dependency
- add logic for handling vsock_loopback setup for already existing
  namespaces
---
 net/vmw_vsock/vsock_loopback.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
index d3ac056663ea..e62f6c516992 100644
--- a/net/vmw_vsock/vsock_loopback.c
+++ b/net/vmw_vsock/vsock_loopback.c
@@ -32,6 +32,9 @@ static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
 	struct vsock_loopback *vsock = &the_vsock_loopback;
 	int len = skb->len;
 
+	virtio_vsock_skb_set_net(skb, net);
+	virtio_vsock_skb_set_net_mode(skb, net_mode);
+
 	virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
 	queue_work(vsock->workqueue, &vsock->pkt_work);
 
@@ -116,8 +119,10 @@ static void vsock_loopback_work(struct work_struct *work)
 {
 	struct vsock_loopback *vsock =
 		container_of(work, struct vsock_loopback, pkt_work);
+	enum vsock_net_mode net_mode;
 	struct sk_buff_head pkts;
 	struct sk_buff *skb;
+	struct net *net;
 
 	skb_queue_head_init(&pkts);
 
@@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
 		 */
 		virtio_transport_consume_skb_sent(skb, false);
 		virtio_transport_deliver_tap_pkt(skb);
-		virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
+
+		/* In the case of virtio_transport_reset_no_sock(), the skb
+		 * does not hold a reference on the socket, and so does not
+		 * transitively hold a reference on the net.
+		 *
+		 * There is an ABA race condition in this sequence:
+		 * 1. the sender sends a packet
+		 * 2. worker calls virtio_transport_recv_pkt(), using the
+		 *    sender's net
+		 * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
+		 *    sender's net
+		 * 4. virtio_transport_recv_pkt() free's the skb, dropping the
+		 *    reference to the socket
+		 * 5. the socket closes, frees its reference to the net
+		 * 6. Finally, the worker for the second t->send_pkt() call
+		 *    processes the skb, and uses the now stale net pointer for
+		 *    socket lookups.
+		 *
+		 * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
+		 * and hold it until virtio_transport_recv_pkt() completes.
+		 *
+		 * Additionally, we must grab a reference on the skb before
+		 * calling virtio_transport_recv_pkt() to prevent it from
+		 * freeing the skb before we have a chance to release the net.
+		 */
+		net_mode = virtio_vsock_skb_net_mode(skb);
+		net = virtio_vsock_skb_net(skb);
+
+		skb_get(skb);
+
+		virtio_transport_recv_pkt(&loopback_transport, skb, net,
+					  net_mode);
+
+		virtio_vsock_skb_clear_net(skb);
+		kfree_skb(skb);
 	}
 }
 

-- 
2.47.3

On Tue, Nov 11, 2025 at 10:54:48PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add NS support to vsock loopback. Sockets in a global mode netns
>communicate with each other, regardless of namespace. Sockets in a local
>mode netns may only communicate with other sockets within the same
>namespace.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v9:
>- remove per-netns vsock_loopback and workqueues, just re-using
>  the net and net_mode in skb->cb achieved the same result in a simpler
>  way. Also removed need for pernet_subsys.
>- properly track net references
>
>Changes in v7:
>- drop for_each_net() init/exit, drop net_rwsem, the pernet registration
>  handles this automatically and race-free
>- flush workqueue before destruction, purge pkt list
>- remember net_mode instead of current net mode
>- keep space after INIT_WORK()
>- change vsock_loopback in netns_vsock to ->priv void ptr
>- rename `orig_net_mode` to `net_mode`
>- remove useless comment
>- protect `register_pernet_subsys()` with `net_rwsem`
>- do cleanup before releasing `net_rwsem` when failure happens
>- call `unregister_pernet_subsys()` in `vsock_loopback_exit()`
>- call `vsock_loopback_deinit_vsock()` in `vsock_loopback_exit()`
>
>Changes in v6:
>- init pernet ops for vsock_loopback module
>- vsock_loopback: add space in struct to clarify lock protection
>- do proper cleanup/unregister on vsock_loopback_exit()
>- vsock_loopback: use virtio_vsock_skb_net()
>
>Changes in v5:
>- add callbacks code to avoid reverse dependency
>- add logic for handling vsock_loopback setup for already existing
>  namespaces
>---
> net/vmw_vsock/vsock_loopback.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index d3ac056663ea..e62f6c516992 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -32,6 +32,9 @@ static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
> 	struct vsock_loopback *vsock = &the_vsock_loopback;
> 	int len = skb->len;
>
>+	virtio_vsock_skb_set_net(skb, net);
>+	virtio_vsock_skb_set_net_mode(skb, net_mode);
>+
> 	virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
> 	queue_work(vsock->workqueue, &vsock->pkt_work);
>
>@@ -116,8 +119,10 @@ static void vsock_loopback_work(struct work_struct *work)
> {
> 	struct vsock_loopback *vsock =
> 		container_of(work, struct vsock_loopback, pkt_work);
>+	enum vsock_net_mode net_mode;
> 	struct sk_buff_head pkts;
> 	struct sk_buff *skb;
>+	struct net *net;
>
> 	skb_queue_head_init(&pkts);
>
>@@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
> 		 */
> 		virtio_transport_consume_skb_sent(skb, false);
> 		virtio_transport_deliver_tap_pkt(skb);
>-		virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
>+
>+		/* In the case of virtio_transport_reset_no_sock(), the skb
>+		 * does not hold a reference on the socket, and so does not
>+		 * transitively hold a reference on the net.
>+		 *
>+		 * There is an ABA race condition in this sequence:
>+		 * 1. the sender sends a packet
>+		 * 2. worker calls virtio_transport_recv_pkt(), using the
>+		 *    sender's net
>+		 * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
>+		 *    sender's net
>+		 * 4. virtio_transport_recv_pkt() free's the skb, dropping the
>+		 *    reference to the socket
>+		 * 5. the socket closes, frees its reference to the net
>+		 * 6. Finally, the worker for the second t->send_pkt() call
>+		 *    processes the skb, and uses the now stale net pointer for
>+		 *    socket lookups.
>+		 *
>+		 * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
>+		 * and hold it until virtio_transport_recv_pkt() completes.
>+		 *
>+		 * Additionally, we must grab a reference on the skb before
>+		 * calling virtio_transport_recv_pkt() to prevent it from
>+		 * freeing the skb before we have a chance to release the net.
>+		 */
>+		net_mode = virtio_vsock_skb_net_mode(skb);
>+		net = virtio_vsock_skb_net(skb);

Wait, we are adding those just for loopback (in theory used only for
testing/debugging)? And only to support virtio_transport_reset_no_sock() 
use case?

Honestly I don't like this, do we have any alternative?

I'll also try to think something else.

Stefano

>+
>+		skb_get(skb);
>+
>+		virtio_transport_recv_pkt(&loopback_transport, skb, net,
>+					  net_mode);
>+
>+		virtio_vsock_skb_clear_net(skb);
>+		kfree_skb(skb);
> 	}
> }
>
>
>-- 
>2.47.3
>

On Wed, Nov 12, 2025 at 03:19:47PM +0100, Stefano Garzarella wrote:
> On Tue, Nov 11, 2025 at 10:54:48PM -0800, Bobby Eshleman wrote:
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> > 
> > Add NS support to vsock loopback. Sockets in a global mode netns
> > communicate with each other, regardless of namespace. Sockets in a local
> > mode netns may only communicate with other sockets within the same
> > namespace.
> > 
> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
> > ---
> > Changes in v9:
> > - remove per-netns vsock_loopback and workqueues, just re-using
> >  the net and net_mode in skb->cb achieved the same result in a simpler
> >  way. Also removed need for pernet_subsys.
> > - properly track net references
> > 
> > Changes in v7:
> > - drop for_each_net() init/exit, drop net_rwsem, the pernet registration
> >  handles this automatically and race-free
> > - flush workqueue before destruction, purge pkt list
> > - remember net_mode instead of current net mode
> > - keep space after INIT_WORK()
> > - change vsock_loopback in netns_vsock to ->priv void ptr
> > - rename `orig_net_mode` to `net_mode`
> > - remove useless comment
> > - protect `register_pernet_subsys()` with `net_rwsem`
> > - do cleanup before releasing `net_rwsem` when failure happens
> > - call `unregister_pernet_subsys()` in `vsock_loopback_exit()`
> > - call `vsock_loopback_deinit_vsock()` in `vsock_loopback_exit()`
> > 
> > Changes in v6:
> > - init pernet ops for vsock_loopback module
> > - vsock_loopback: add space in struct to clarify lock protection
> > - do proper cleanup/unregister on vsock_loopback_exit()
> > - vsock_loopback: use virtio_vsock_skb_net()
> > 
> > Changes in v5:
> > - add callbacks code to avoid reverse dependency
> > - add logic for handling vsock_loopback setup for already existing
> >  namespaces
> > ---
> > net/vmw_vsock/vsock_loopback.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 40 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> > index d3ac056663ea..e62f6c516992 100644
> > --- a/net/vmw_vsock/vsock_loopback.c
> > +++ b/net/vmw_vsock/vsock_loopback.c
> > @@ -32,6 +32,9 @@ static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
> > 	struct vsock_loopback *vsock = &the_vsock_loopback;
> > 	int len = skb->len;
> > 
> > +	virtio_vsock_skb_set_net(skb, net);
> > +	virtio_vsock_skb_set_net_mode(skb, net_mode);
> > +
> > 	virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
> > 	queue_work(vsock->workqueue, &vsock->pkt_work);
> > 
> > @@ -116,8 +119,10 @@ static void vsock_loopback_work(struct work_struct *work)
> > {
> > 	struct vsock_loopback *vsock =
> > 		container_of(work, struct vsock_loopback, pkt_work);
> > +	enum vsock_net_mode net_mode;
> > 	struct sk_buff_head pkts;
> > 	struct sk_buff *skb;
> > +	struct net *net;
> > 
> > 	skb_queue_head_init(&pkts);
> > 
> > @@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
> > 		 */
> > 		virtio_transport_consume_skb_sent(skb, false);
> > 		virtio_transport_deliver_tap_pkt(skb);
> > -		virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
> > +
> > +		/* In the case of virtio_transport_reset_no_sock(), the skb
> > +		 * does not hold a reference on the socket, and so does not
> > +		 * transitively hold a reference on the net.
> > +		 *
> > +		 * There is an ABA race condition in this sequence:
> > +		 * 1. the sender sends a packet
> > +		 * 2. worker calls virtio_transport_recv_pkt(), using the
> > +		 *    sender's net
> > +		 * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
> > +		 *    sender's net
> > +		 * 4. virtio_transport_recv_pkt() free's the skb, dropping the
> > +		 *    reference to the socket
> > +		 * 5. the socket closes, frees its reference to the net
> > +		 * 6. Finally, the worker for the second t->send_pkt() call
> > +		 *    processes the skb, and uses the now stale net pointer for
> > +		 *    socket lookups.
> > +		 *
> > +		 * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
> > +		 * and hold it until virtio_transport_recv_pkt() completes.
> > +		 *
> > +		 * Additionally, we must grab a reference on the skb before
> > +		 * calling virtio_transport_recv_pkt() to prevent it from
> > +		 * freeing the skb before we have a chance to release the net.
> > +		 */
> > +		net_mode = virtio_vsock_skb_net_mode(skb);
> > +		net = virtio_vsock_skb_net(skb);
> 
> Wait, we are adding those just for loopback (in theory used only for
> testing/debugging)? And only to support virtio_transport_reset_no_sock() use
> case?

Yes, exactly, only loopback + reset_no_sock(). The issue doesn't exist
for vhost-vsock because vhost_vsock holds a net reference, and it
doesn't exist for non-reset_no_sock calls because after looking up the
socket we transfer skb ownership to it, which holds down the skb -> sk ->
net reference chain.

> 
> Honestly I don't like this, do we have any alternative?
> 
> I'll also try to think something else.
> 
> Stefano


I've been thinking about this all morning... maybe
we can do something like this:

```

virtio_transport_recv_pkt(...,  struct sock *reply_sk) {... }

virtio_transport_reset_no_sock(..., reply_sk)
{
	if (reply_sk)
		skb_set_owner_sk_safe(reply, reply_sk)

	t->send_pkt(reply);
}

vsock_loopback_work(...)
{
	virtio_transport_recv_pkt(..., skb, skb->sk);
}


for other transports:

	virtio_transport_recv_pkt(..., skb, NULL);

```

This way 'reply' keeps the sk and sk->net alive even after
virtio_transport_recv_pkt() frees 'skb'. The net won't be released until
after 'reply' is freed back on the other side, removing the race.

It makes semantic sense too... for loopback, we already know which sk
the reply is going back to. For other transports, we don't because
they're across the virt boundary.

WDYT?

I hate to suggest this, but another option might be to just do nothing?
In order for this race to have any real effect, a loopback socket must
send a pkt to a non-existent socket, immediately close(), then the
namespace deleted, a new namespace created with the same pointer
address, and finally a new socket with the same port created in that
namespace, all before the reply RST reaches recv_pkt()... at which point
the newly created socket would wrongfully receive the RST.

Best,
Bobby

On Wed, Nov 12, 2025 at 10:27:18AM -0800, Bobby Eshleman wrote:
>On Wed, Nov 12, 2025 at 03:19:47PM +0100, Stefano Garzarella wrote:
>> On Tue, Nov 11, 2025 at 10:54:48PM -0800, Bobby Eshleman wrote:
>> > From: Bobby Eshleman <bobbyeshleman@meta.com>
>> >
>> > Add NS support to vsock loopback. Sockets in a global mode netns
>> > communicate with each other, regardless of namespace. Sockets in a local
>> > mode netns may only communicate with other sockets within the same
>> > namespace.
>> >
>> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>> > ---
>> > Changes in v9:
>> > - remove per-netns vsock_loopback and workqueues, just re-using
>> >  the net and net_mode in skb->cb achieved the same result in a simpler
>> >  way. Also removed need for pernet_subsys.
>> > - properly track net references
>> >
>> > Changes in v7:
>> > - drop for_each_net() init/exit, drop net_rwsem, the pernet registration
>> >  handles this automatically and race-free
>> > - flush workqueue before destruction, purge pkt list
>> > - remember net_mode instead of current net mode
>> > - keep space after INIT_WORK()
>> > - change vsock_loopback in netns_vsock to ->priv void ptr
>> > - rename `orig_net_mode` to `net_mode`
>> > - remove useless comment
>> > - protect `register_pernet_subsys()` with `net_rwsem`
>> > - do cleanup before releasing `net_rwsem` when failure happens
>> > - call `unregister_pernet_subsys()` in `vsock_loopback_exit()`
>> > - call `vsock_loopback_deinit_vsock()` in `vsock_loopback_exit()`
>> >
>> > Changes in v6:
>> > - init pernet ops for vsock_loopback module
>> > - vsock_loopback: add space in struct to clarify lock protection
>> > - do proper cleanup/unregister on vsock_loopback_exit()
>> > - vsock_loopback: use virtio_vsock_skb_net()
>> >
>> > Changes in v5:
>> > - add callbacks code to avoid reverse dependency
>> > - add logic for handling vsock_loopback setup for already existing
>> >  namespaces
>> > ---
>> > net/vmw_vsock/vsock_loopback.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>> > 1 file changed, 40 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>> > index d3ac056663ea..e62f6c516992 100644
>> > --- a/net/vmw_vsock/vsock_loopback.c
>> > +++ b/net/vmw_vsock/vsock_loopback.c
>> > @@ -32,6 +32,9 @@ static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
>> > 	struct vsock_loopback *vsock = &the_vsock_loopback;
>> > 	int len = skb->len;
>> >
>> > +	virtio_vsock_skb_set_net(skb, net);
>> > +	virtio_vsock_skb_set_net_mode(skb, net_mode);
>> > +
>> > 	virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
>> > 	queue_work(vsock->workqueue, &vsock->pkt_work);
>> >
>> > @@ -116,8 +119,10 @@ static void vsock_loopback_work(struct work_struct *work)
>> > {
>> > 	struct vsock_loopback *vsock =
>> > 		container_of(work, struct vsock_loopback, pkt_work);
>> > +	enum vsock_net_mode net_mode;
>> > 	struct sk_buff_head pkts;
>> > 	struct sk_buff *skb;
>> > +	struct net *net;
>> >
>> > 	skb_queue_head_init(&pkts);
>> >
>> > @@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
>> > 		 */
>> > 		virtio_transport_consume_skb_sent(skb, false);
>> > 		virtio_transport_deliver_tap_pkt(skb);
>> > -		virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
>> > +
>> > +		/* In the case of virtio_transport_reset_no_sock(), the skb
>> > +		 * does not hold a reference on the socket, and so does not
>> > +		 * transitively hold a reference on the net.
>> > +		 *
>> > +		 * There is an ABA race condition in this sequence:
>> > +		 * 1. the sender sends a packet
>> > +		 * 2. worker calls virtio_transport_recv_pkt(), using the
>> > +		 *    sender's net
>> > +		 * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
>> > +		 *    sender's net
>> > +		 * 4. virtio_transport_recv_pkt() free's the skb, dropping the
>> > +		 *    reference to the socket
>> > +		 * 5. the socket closes, frees its reference to the net
>> > +		 * 6. Finally, the worker for the second t->send_pkt() call
>> > +		 *    processes the skb, and uses the now stale net pointer for
>> > +		 *    socket lookups.
>> > +		 *
>> > +		 * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
>> > +		 * and hold it until virtio_transport_recv_pkt() completes.
>> > +		 *
>> > +		 * Additionally, we must grab a reference on the skb before
>> > +		 * calling virtio_transport_recv_pkt() to prevent it from
>> > +		 * freeing the skb before we have a chance to release the net.
>> > +		 */
>> > +		net_mode = virtio_vsock_skb_net_mode(skb);
>> > +		net = virtio_vsock_skb_net(skb);
>>
>> Wait, we are adding those just for loopback (in theory used only for
>> testing/debugging)? And only to support virtio_transport_reset_no_sock() use
>> case?
>
>Yes, exactly, only loopback + reset_no_sock(). The issue doesn't exist
>for vhost-vsock because vhost_vsock holds a net reference, and it
>doesn't exist for non-reset_no_sock calls because after looking up the
>socket we transfer skb ownership to it, which holds down the skb -> sk ->
>net reference chain.
>
>>
>> Honestly I don't like this, do we have any alternative?
>>
>> I'll also try to think something else.
>>
>> Stefano
>
>
>I've been thinking about this all morning... maybe
>we can do something like this:
>
>```
>
>virtio_transport_recv_pkt(...,  struct sock *reply_sk) {... }
>
>virtio_transport_reset_no_sock(..., reply_sk)
>{
>	if (reply_sk)
>		skb_set_owner_sk_safe(reply, reply_sk)

Interesting, but what about if we call skb_set_owner_sk_safe() in 
vsock_loopback.c just before calling virtio_transport_recv_pkt() for 
every skb?

Maybe we should refactor a bit virtio_transport_recv_pkt() e.g. moving 
`skb_set_owner_sk_safe()` to be sure it's called only when we are sure 
it's the right socket (e.g. after checking SOCK_DONE).

WDYT?

>
>	t->send_pkt(reply);
>}
>
>vsock_loopback_work(...)
>{
>	virtio_transport_recv_pkt(..., skb, skb->sk);
>}
>
>
>for other transports:
>
>	virtio_transport_recv_pkt(..., skb, NULL);
>
>```
>
>This way 'reply' keeps the sk and sk->net alive even after
>virtio_transport_recv_pkt() frees 'skb'. The net won't be released until
>after 'reply' is freed back on the other side, removing the race.
>
>It makes semantic sense too... for loopback, we already know which sk
>the reply is going back to. For other transports, we don't because
>they're across the virt boundary.
>
>WDYT?
>
>I hate to suggest this, but another option might be to just do nothing?
>In order for this race to have any real effect, a loopback socket must
>send a pkt to a non-existent socket, immediately close(), then the
>namespace deleted, a new namespace created with the same pointer
>address, and finally a new socket with the same port created in that
>namespace, all before the reply RST reaches recv_pkt()... at which point
>the newly created socket would wrongfully receive the RST.

Yeah, let's keep this as plan B for now :-)

Thanks,
Stefano

On Thu, Nov 13, 2025 at 04:24:44PM +0100, Stefano Garzarella wrote:
> On Wed, Nov 12, 2025 at 10:27:18AM -0800, Bobby Eshleman wrote:
> > On Wed, Nov 12, 2025 at 03:19:47PM +0100, Stefano Garzarella wrote:
> > > On Tue, Nov 11, 2025 at 10:54:48PM -0800, Bobby Eshleman wrote:
> > > > From: Bobby Eshleman <bobbyeshleman@meta.com>
> > > >
> > > > Add NS support to vsock loopback. Sockets in a global mode netns
> > > > communicate with each other, regardless of namespace. Sockets in a local
> > > > mode netns may only communicate with other sockets within the same
> > > > namespace.
> > > >
> > > > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>

[...]

> > > > @@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
> > > > 		 */
> > > > 		virtio_transport_consume_skb_sent(skb, false);
> > > > 		virtio_transport_deliver_tap_pkt(skb);
> > > > -		virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
> > > > +
> > > > +		/* In the case of virtio_transport_reset_no_sock(), the skb
> > > > +		 * does not hold a reference on the socket, and so does not
> > > > +		 * transitively hold a reference on the net.
> > > > +		 *
> > > > +		 * There is an ABA race condition in this sequence:
> > > > +		 * 1. the sender sends a packet
> > > > +		 * 2. worker calls virtio_transport_recv_pkt(), using the
> > > > +		 *    sender's net
> > > > +		 * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
> > > > +		 *    sender's net
> > > > +		 * 4. virtio_transport_recv_pkt() free's the skb, dropping the
> > > > +		 *    reference to the socket
> > > > +		 * 5. the socket closes, frees its reference to the net
> > > > +		 * 6. Finally, the worker for the second t->send_pkt() call
> > > > +		 *    processes the skb, and uses the now stale net pointer for
> > > > +		 *    socket lookups.
> > > > +		 *
> > > > +		 * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
> > > > +		 * and hold it until virtio_transport_recv_pkt() completes.
> > > > +		 *
> > > > +		 * Additionally, we must grab a reference on the skb before
> > > > +		 * calling virtio_transport_recv_pkt() to prevent it from
> > > > +		 * freeing the skb before we have a chance to release the net.
> > > > +		 */
> > > > +		net_mode = virtio_vsock_skb_net_mode(skb);
> > > > +		net = virtio_vsock_skb_net(skb);
> > > 
> > > Wait, we are adding those just for loopback (in theory used only for
> > > testing/debugging)? And only to support virtio_transport_reset_no_sock() use
> > > case?
> > 
> > Yes, exactly, only loopback + reset_no_sock(). The issue doesn't exist
> > for vhost-vsock because vhost_vsock holds a net reference, and it
> > doesn't exist for non-reset_no_sock calls because after looking up the
> > socket we transfer skb ownership to it, which holds down the skb -> sk ->
> > net reference chain.
> > 
> > > 
> > > Honestly I don't like this, do we have any alternative?
> > > 
> > > I'll also try to think something else.
> > > 
> > > Stefano
> > 
> > 
> > I've been thinking about this all morning... maybe
> > we can do something like this:
> > 
> > ```
> > 
> > virtio_transport_recv_pkt(...,  struct sock *reply_sk) {... }
> > 
> > virtio_transport_reset_no_sock(..., reply_sk)
> > {
> > 	if (reply_sk)
> > 		skb_set_owner_sk_safe(reply, reply_sk)
> 
> Interesting, but what about if we call skb_set_owner_sk_safe() in
> vsock_loopback.c just before calling virtio_transport_recv_pkt() for every
> skb?

I think the issue with this is that at the time vsock_loopback calls
virtio_transport_recv_pkt() the reply skb hasn't yet been allocated by
virtio_transport_reset_no_sock() and we can't wait for it to return
because the original skb may be freed by then.

We might be able to keep it all in vsock_loopback if we removed the need
to use the original skb or sk by just using the net. But to do that we
would need to add a netns_tracker per net somewhere. I guess that would
end up in a list or hashmap in struct vsock_loopback.

Another option that does simplify a little, but unfortunately still doesn't keep
everything in loopback:

@@ -1205,7 +1205,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
 	if (!reply)
 		return -ENOMEM;
 
-	return t->send_pkt(reply, net, net_mode);
+	return t->send_pkt(reply, net, net_mode, skb->sk);
 }

@@ -27,11 +27,16 @@ static u32 vsock_loopback_get_local_cid(void)
 }

 static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
-				   enum vsock_net_mode net_mode)
+				   enum vsock_net_mode net_mode,
+				   struct sock *rst_owner)
 {
 	struct vsock_loopback *vsock = &the_vsock_loopback;
 	int len = skb->len;
 
+	if (!skb->sk && rst_owner)
+		WARN_ONCE(!skb_set_owner_sk_safe(skb, rst_owner),
+			  "loopback socket has sk_refcnt == 0\n");
+
 	virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
 	queue_work(vsock->workqueue, &vsock->pkt_work);

> 
> Maybe we should refactor a bit virtio_transport_recv_pkt() e.g. moving
> `skb_set_owner_sk_safe()` to be sure it's called only when we are sure it's
> the right socket (e.g. after checking SOCK_DONE).
> 
> WDYT?

I agree, it is called a little prematurely.

Thanks,
Bobby

On Thu, Nov 13, 2025 at 10:26:04AM -0800, Bobby Eshleman wrote:
>On Thu, Nov 13, 2025 at 04:24:44PM +0100, Stefano Garzarella wrote:
>> On Wed, Nov 12, 2025 at 10:27:18AM -0800, Bobby Eshleman wrote:
>> > On Wed, Nov 12, 2025 at 03:19:47PM +0100, Stefano Garzarella wrote:
>> > > On Tue, Nov 11, 2025 at 10:54:48PM -0800, Bobby Eshleman wrote:
>> > > > From: Bobby Eshleman <bobbyeshleman@meta.com>
>> > > >
>> > > > Add NS support to vsock loopback. Sockets in a global mode netns
>> > > > communicate with each other, regardless of namespace. Sockets in a local
>> > > > mode netns may only communicate with other sockets within the same
>> > > > namespace.
>> > > >
>> > > > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>
>[...]
>
>> > > > @@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
>> > > > 		 */
>> > > > 		virtio_transport_consume_skb_sent(skb, false);
>> > > > 		virtio_transport_deliver_tap_pkt(skb);
>> > > > -		virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
>> > > > +
>> > > > +		/* In the case of virtio_transport_reset_no_sock(), the skb
>> > > > +		 * does not hold a reference on the socket, and so does not
>> > > > +		 * transitively hold a reference on the net.
>> > > > +		 *
>> > > > +		 * There is an ABA race condition in this sequence:
>> > > > +		 * 1. the sender sends a packet
>> > > > +		 * 2. worker calls virtio_transport_recv_pkt(), using the
>> > > > +		 *    sender's net
>> > > > +		 * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
>> > > > +		 *    sender's net
>> > > > +		 * 4. virtio_transport_recv_pkt() free's the skb, dropping the
>> > > > +		 *    reference to the socket
>> > > > +		 * 5. the socket closes, frees its reference to the net
>> > > > +		 * 6. Finally, the worker for the second t->send_pkt() call
>> > > > +		 *    processes the skb, and uses the now stale net pointer for
>> > > > +		 *    socket lookups.
>> > > > +		 *
>> > > > +		 * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
>> > > > +		 * and hold it until virtio_transport_recv_pkt() completes.
>> > > > +		 *
>> > > > +		 * Additionally, we must grab a reference on the skb before
>> > > > +		 * calling virtio_transport_recv_pkt() to prevent it from
>> > > > +		 * freeing the skb before we have a chance to release the net.
>> > > > +		 */
>> > > > +		net_mode = virtio_vsock_skb_net_mode(skb);
>> > > > +		net = virtio_vsock_skb_net(skb);
>> > >
>> > > Wait, we are adding those just for loopback (in theory used only for
>> > > testing/debugging)? And only to support virtio_transport_reset_no_sock() use
>> > > case?
>> >
>> > Yes, exactly, only loopback + reset_no_sock(). The issue doesn't exist
>> > for vhost-vsock because vhost_vsock holds a net reference, and it
>> > doesn't exist for non-reset_no_sock calls because after looking up the
>> > socket we transfer skb ownership to it, which holds down the skb -> sk ->
>> > net reference chain.
>> >
>> > >
>> > > Honestly I don't like this, do we have any alternative?
>> > >
>> > > I'll also try to think something else.
>> > >
>> > > Stefano
>> >
>> >
>> > I've been thinking about this all morning... maybe
>> > we can do something like this:
>> >
>> > ```
>> >
>> > virtio_transport_recv_pkt(...,  struct sock *reply_sk) {... }
>> >
>> > virtio_transport_reset_no_sock(..., reply_sk)
>> > {
>> > 	if (reply_sk)
>> > 		skb_set_owner_sk_safe(reply, reply_sk)
>>
>> Interesting, but what about if we call skb_set_owner_sk_safe() in
>> vsock_loopback.c just before calling virtio_transport_recv_pkt() for every
>> skb?
>
>I think the issue with this is that at the time vsock_loopback calls
>virtio_transport_recv_pkt() the reply skb hasn't yet been allocated by
>virtio_transport_reset_no_sock() and we can't wait for it to return
>because the original skb may be freed by then.

Right!

>
>We might be able to keep it all in vsock_loopback if we removed the need
>to use the original skb or sk by just using the net. But to do that we
>would need to add a netns_tracker per net somewhere. I guess that would
>end up in a list or hashmap in struct vsock_loopback.
>
>Another option that does simplify a little, but unfortunately still doesn't keep
>everything in loopback:
>
>@@ -1205,7 +1205,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> 	if (!reply)
> 		return -ENOMEM;
>
>-	return t->send_pkt(reply, net, net_mode);
>+	return t->send_pkt(reply, net, net_mode, skb->sk);
> }
>
>@@ -27,11 +27,16 @@ static u32 vsock_loopback_get_local_cid(void)
> }
>
> static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
>-				   enum vsock_net_mode net_mode)
>+				   enum vsock_net_mode net_mode,
>+				   struct sock *rst_owner)
> {
> 	struct vsock_loopback *vsock = &the_vsock_loopback;
> 	int len = skb->len;
>
>+	if (!skb->sk && rst_owner)
>+		WARN_ONCE(!skb_set_owner_sk_safe(skb, rst_owner),
>+			  "loopback socket has sk_refcnt == 0\n");
>+

This doesn't seem too bad IMO, but at this point, why we can't do that
in virtio_transport_reset_no_sock() for any kind of transport?

I mean, in any case the RST packet should be handled by the same net of 
the "sender", no?

At this point, can we just put the `vsk` of the sender in the `info` and 
virtio_transport_alloc_skb() will already do that.

WDYT?
Am I missing something?

> 	virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
> 	queue_work(vsock->workqueue, &vsock->pkt_work);
>
>>
>> Maybe we should refactor a bit virtio_transport_recv_pkt() e.g. moving
>> `skb_set_owner_sk_safe()` to be sure it's called only when we are sure it's
>> the right socket (e.g. after checking SOCK_DONE).
>>
>> WDYT?
>
>I agree, it is called a little prematurely.

Yep, but I'll leave this out for now :-)

Thanks,
Stefano

On Fri, Nov 14, 2025 at 10:33:42AM +0100, Stefano Garzarella wrote:
> On Thu, Nov 13, 2025 at 10:26:04AM -0800, Bobby Eshleman wrote:
> > On Thu, Nov 13, 2025 at 04:24:44PM +0100, Stefano Garzarella wrote:
> > > On Wed, Nov 12, 2025 at 10:27:18AM -0800, Bobby Eshleman wrote:
> > > > On Wed, Nov 12, 2025 at 03:19:47PM +0100, Stefano Garzarella wrote:
> > > > > On Tue, Nov 11, 2025 at 10:54:48PM -0800, Bobby Eshleman wrote:
> > > > > > From: Bobby Eshleman <bobbyeshleman@meta.com>
> > > > > >
> > > > > > Add NS support to vsock loopback. Sockets in a global mode netns
> > > > > > communicate with each other, regardless of namespace. Sockets in a local
> > > > > > mode netns may only communicate with other sockets within the same
> > > > > > namespace.
> > > > > >
> > > > > > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
> > 
> > [...]
> > 
> > > > > > @@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
> > > > > > 		 */
> > > > > > 		virtio_transport_consume_skb_sent(skb, false);
> > > > > > 		virtio_transport_deliver_tap_pkt(skb);
> > > > > > -		virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
> > > > > > +
> > > > > > +		/* In the case of virtio_transport_reset_no_sock(), the skb
> > > > > > +		 * does not hold a reference on the socket, and so does not
> > > > > > +		 * transitively hold a reference on the net.
> > > > > > +		 *
> > > > > > +		 * There is an ABA race condition in this sequence:
> > > > > > +		 * 1. the sender sends a packet
> > > > > > +		 * 2. worker calls virtio_transport_recv_pkt(), using the
> > > > > > +		 *    sender's net
> > > > > > +		 * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
> > > > > > +		 *    sender's net
> > > > > > +		 * 4. virtio_transport_recv_pkt() free's the skb, dropping the
> > > > > > +		 *    reference to the socket
> > > > > > +		 * 5. the socket closes, frees its reference to the net
> > > > > > +		 * 6. Finally, the worker for the second t->send_pkt() call
> > > > > > +		 *    processes the skb, and uses the now stale net pointer for
> > > > > > +		 *    socket lookups.
> > > > > > +		 *
> > > > > > +		 * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
> > > > > > +		 * and hold it until virtio_transport_recv_pkt() completes.
> > > > > > +		 *
> > > > > > +		 * Additionally, we must grab a reference on the skb before
> > > > > > +		 * calling virtio_transport_recv_pkt() to prevent it from
> > > > > > +		 * freeing the skb before we have a chance to release the net.
> > > > > > +		 */
> > > > > > +		net_mode = virtio_vsock_skb_net_mode(skb);
> > > > > > +		net = virtio_vsock_skb_net(skb);
> > > > >
> > > > > Wait, we are adding those just for loopback (in theory used only for
> > > > > testing/debugging)? And only to support virtio_transport_reset_no_sock() use
> > > > > case?
> > > >
> > > > Yes, exactly, only loopback + reset_no_sock(). The issue doesn't exist
> > > > for vhost-vsock because vhost_vsock holds a net reference, and it
> > > > doesn't exist for non-reset_no_sock calls because after looking up the
> > > > socket we transfer skb ownership to it, which holds down the skb -> sk ->
> > > > net reference chain.
> > > >
> > > > >
> > > > > Honestly I don't like this, do we have any alternative?
> > > > >
> > > > > I'll also try to think something else.
> > > > >
> > > > > Stefano
> > > >
> > > >
> > > > I've been thinking about this all morning... maybe
> > > > we can do something like this:
> > > >
> > > > ```
> > > >
> > > > virtio_transport_recv_pkt(...,  struct sock *reply_sk) {... }
> > > >
> > > > virtio_transport_reset_no_sock(..., reply_sk)
> > > > {
> > > > 	if (reply_sk)
> > > > 		skb_set_owner_sk_safe(reply, reply_sk)
> > > 
> > > Interesting, but what about if we call skb_set_owner_sk_safe() in
> > > vsock_loopback.c just before calling virtio_transport_recv_pkt() for every
> > > skb?
> > 
> > I think the issue with this is that at the time vsock_loopback calls
> > virtio_transport_recv_pkt() the reply skb hasn't yet been allocated by
> > virtio_transport_reset_no_sock() and we can't wait for it to return
> > because the original skb may be freed by then.
> 
> Right!
> 
> > 
> > We might be able to keep it all in vsock_loopback if we removed the need
> > to use the original skb or sk by just using the net. But to do that we
> > would need to add a netns_tracker per net somewhere. I guess that would
> > end up in a list or hashmap in struct vsock_loopback.
> > 
> > Another option that does simplify a little, but unfortunately still doesn't keep
> > everything in loopback:
> > 
> > @@ -1205,7 +1205,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> > 	if (!reply)
> > 		return -ENOMEM;
> > 
> > -	return t->send_pkt(reply, net, net_mode);
> > +	return t->send_pkt(reply, net, net_mode, skb->sk);
> > }
> > 
> > @@ -27,11 +27,16 @@ static u32 vsock_loopback_get_local_cid(void)
> > }
> > 
> > static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
> > -				   enum vsock_net_mode net_mode)
> > +				   enum vsock_net_mode net_mode,
> > +				   struct sock *rst_owner)
> > {
> > 	struct vsock_loopback *vsock = &the_vsock_loopback;
> > 	int len = skb->len;
> > 
> > +	if (!skb->sk && rst_owner)
> > +		WARN_ONCE(!skb_set_owner_sk_safe(skb, rst_owner),
> > +			  "loopback socket has sk_refcnt == 0\n");
> > +
> 
> This doesn't seem too bad IMO, but at this point, why we can't do that
> in virtio_transport_reset_no_sock() for any kind of transport?
> 
> I mean, in any case the RST packet should be handled by the same net of the
> "sender", no?
> 
> At this point, can we just put the `vsk` of the sender in the `info` and
> virtio_transport_alloc_skb() will already do that.
> 
> WDYT?
> Am I missing something?

This is the right answer... I'm pretty sure this works out-of-the-box
for all transports.

I'll implement it and report back with a new rev if all good or come
back to this thread to discuss if any issues arise.

Have a good weekend!

Best,
Bobby

On Fri, Nov 14, 2025 at 02:13:59PM -0800, Bobby Eshleman wrote:
>On Fri, Nov 14, 2025 at 10:33:42AM +0100, Stefano Garzarella wrote:
>> On Thu, Nov 13, 2025 at 10:26:04AM -0800, Bobby Eshleman wrote:
>> > On Thu, Nov 13, 2025 at 04:24:44PM +0100, Stefano Garzarella wrote:
>> > > On Wed, Nov 12, 2025 at 10:27:18AM -0800, Bobby Eshleman wrote:
>> > > > On Wed, Nov 12, 2025 at 03:19:47PM +0100, Stefano Garzarella wrote:
>> > > > > On Tue, Nov 11, 2025 at 10:54:48PM -0800, Bobby Eshleman wrote:
>> > > > > > From: Bobby Eshleman <bobbyeshleman@meta.com>
>> > > > > >
>> > > > > > Add NS support to vsock loopback. Sockets in a global mode netns
>> > > > > > communicate with each other, regardless of namespace. Sockets in a local
>> > > > > > mode netns may only communicate with other sockets within the same
>> > > > > > namespace.
>> > > > > >
>> > > > > > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>> >
>> > [...]
>> >
>> > > > > > @@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
>> > > > > > 		 */
>> > > > > > 		virtio_transport_consume_skb_sent(skb, false);
>> > > > > > 		virtio_transport_deliver_tap_pkt(skb);
>> > > > > > -		virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
>> > > > > > +
>> > > > > > +		/* In the case of virtio_transport_reset_no_sock(), the skb
>> > > > > > +		 * does not hold a reference on the socket, and so does not
>> > > > > > +		 * transitively hold a reference on the net.
>> > > > > > +		 *
>> > > > > > +		 * There is an ABA race condition in this sequence:
>> > > > > > +		 * 1. the sender sends a packet
>> > > > > > +		 * 2. worker calls virtio_transport_recv_pkt(), using the
>> > > > > > +		 *    sender's net
>> > > > > > +		 * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
>> > > > > > +		 *    sender's net
>> > > > > > +		 * 4. virtio_transport_recv_pkt() free's the skb, dropping the
>> > > > > > +		 *    reference to the socket
>> > > > > > +		 * 5. the socket closes, frees its reference to the net
>> > > > > > +		 * 6. Finally, the worker for the second t->send_pkt() call
>> > > > > > +		 *    processes the skb, and uses the now stale net pointer for
>> > > > > > +		 *    socket lookups.
>> > > > > > +		 *
>> > > > > > +		 * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
>> > > > > > +		 * and hold it until virtio_transport_recv_pkt() completes.
>> > > > > > +		 *
>> > > > > > +		 * Additionally, we must grab a reference on the skb before
>> > > > > > +		 * calling virtio_transport_recv_pkt() to prevent it from
>> > > > > > +		 * freeing the skb before we have a chance to release the net.
>> > > > > > +		 */
>> > > > > > +		net_mode = virtio_vsock_skb_net_mode(skb);
>> > > > > > +		net = virtio_vsock_skb_net(skb);
>> > > > >
>> > > > > Wait, we are adding those just for loopback (in theory used only for
>> > > > > testing/debugging)? And only to support virtio_transport_reset_no_sock() use
>> > > > > case?
>> > > >
>> > > > Yes, exactly, only loopback + reset_no_sock(). The issue doesn't exist
>> > > > for vhost-vsock because vhost_vsock holds a net reference, and it
>> > > > doesn't exist for non-reset_no_sock calls because after looking up the
>> > > > socket we transfer skb ownership to it, which holds down the skb -> sk ->
>> > > > net reference chain.
>> > > >
>> > > > >
>> > > > > Honestly I don't like this, do we have any alternative?
>> > > > >
>> > > > > I'll also try to think something else.
>> > > > >
>> > > > > Stefano
>> > > >
>> > > >
>> > > > I've been thinking about this all morning... maybe
>> > > > we can do something like this:
>> > > >
>> > > > ```
>> > > >
>> > > > virtio_transport_recv_pkt(...,  struct sock *reply_sk) {... }
>> > > >
>> > > > virtio_transport_reset_no_sock(..., reply_sk)
>> > > > {
>> > > > 	if (reply_sk)
>> > > > 		skb_set_owner_sk_safe(reply, reply_sk)
>> > >
>> > > Interesting, but what about if we call skb_set_owner_sk_safe() in
>> > > vsock_loopback.c just before calling virtio_transport_recv_pkt() for every
>> > > skb?
>> >
>> > I think the issue with this is that at the time vsock_loopback calls
>> > virtio_transport_recv_pkt() the reply skb hasn't yet been allocated by
>> > virtio_transport_reset_no_sock() and we can't wait for it to return
>> > because the original skb may be freed by then.
>>
>> Right!
>>
>> >
>> > We might be able to keep it all in vsock_loopback if we removed the need
>> > to use the original skb or sk by just using the net. But to do that we
>> > would need to add a netns_tracker per net somewhere. I guess that would
>> > end up in a list or hashmap in struct vsock_loopback.
>> >
>> > Another option that does simplify a little, but unfortunately still doesn't keep
>> > everything in loopback:
>> >
>> > @@ -1205,7 +1205,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
>> > 	if (!reply)
>> > 		return -ENOMEM;
>> >
>> > -	return t->send_pkt(reply, net, net_mode);
>> > +	return t->send_pkt(reply, net, net_mode, skb->sk);
>> > }
>> >
>> > @@ -27,11 +27,16 @@ static u32 vsock_loopback_get_local_cid(void)
>> > }
>> >
>> > static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
>> > -				   enum vsock_net_mode net_mode)
>> > +				   enum vsock_net_mode net_mode,
>> > +				   struct sock *rst_owner)
>> > {
>> > 	struct vsock_loopback *vsock = &the_vsock_loopback;
>> > 	int len = skb->len;
>> >
>> > +	if (!skb->sk && rst_owner)
>> > +		WARN_ONCE(!skb_set_owner_sk_safe(skb, rst_owner),
>> > +			  "loopback socket has sk_refcnt == 0\n");
>> > +
>>
>> This doesn't seem too bad IMO, but at this point, why we can't do that
>> in virtio_transport_reset_no_sock() for any kind of transport?
>>
>> I mean, in any case the RST packet should be handled by the same net of the
>> "sender", no?
>>
>> At this point, can we just put the `vsk` of the sender in the `info` and
>> virtio_transport_alloc_skb() will already do that.
>>
>> WDYT?
>> Am I missing something?
>
>This is the right answer... I'm pretty sure this works out-of-the-box
>for all transports.

I hope too, but I was worried there was something hidden, even though it 
should make sense. Anyway, I would make this change in a separate patch, 
explaining our reasoning clearly (that reset packets should be 
associated with the sending socket, etc.), so that if we find something 
wrong in the future, we can revert it if necessary.

>
>I'll implement it and report back with a new rev if all good or come
>back to this thread to discuss if any issues arise.
>
>Have a good weekend!

I did :-) hope it was good for you too!

Ciao,
Stefano