[PATCH] vsock/virtio: fix memory leak in virtio_transport_recv_listen()

[PATCH] vsock/virtio: fix memory leak in virtio_transport_recv_listen()

[Deepanshu Kartikey]

Two bugs exist in virtio_transport_recv_listen():

1. On the transport assignment error path, sk_acceptq_added() is called
   but sk_acceptq_removed() is never called when vsock_assign_transport()
   fails or assigns a different transport than expected. This causes the
   parent listener's accept backlog counter to be permanently inflated,
   eventually causing sk_acceptq_is_full() to reject legitimate incoming
   connections.

2. There is a race between __vsock_release() and vsock_enqueue_accept().
   __vsock_release() sets sk->sk_shutdown to SHUTDOWN_MASK and flushes
   the accept queue under the parent socket lock. However,
   virtio_transport_recv_listen() checks sk_shutdown and subsequently
   calls vsock_enqueue_accept() without holding the parent socket lock.
   This means a child socket can be enqueued after __vsock_release() has
   already flushed the queue, causing the child socket and its associated
   resources to leak
   permanently. The existing comment in the code hints at this race but
   the fix was never implemented.

Fix both issues: add sk_acceptq_removed() on the transport error path,
and re-check sk->sk_shutdown under the parent socket lock before calling
vsock_enqueue_accept() to close the race window. The child socket lock
is released before acquiring the parent socket lock to maintain correct
lock ordering (parent before child).

Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
Tested-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 net/vmw_vsock/virtio_transport_common.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 416d533f493d..fad5fa4a4296 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1578,6 +1578,7 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
 	 */
 	if (ret || vchild->transport != &t->transport) {
 		release_sock(child);
+		sk_acceptq_removed(sk);
 		virtio_transport_reset_no_sock(t, skb, sock_net(sk));
 		sock_put(child);
 		return ret;
@@ -1588,11 +1589,19 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
 		child->sk_write_space(child);
 
 	vsock_insert_connected(vchild);
+	release_sock(child);
+	lock_sock(sk);
+	if (sk->sk_shutdown == SHUTDOWN_MASK) {
+		release_sock(sk);
+		sk_acceptq_removed(sk);
+		virtio_transport_reset_no_sock(t, skb, sock_net(sk));
+		sock_put(child);
+		return -ESHUTDOWN;
+	}
 	vsock_enqueue_accept(sk, child);
+	release_sock(sk);
 	virtio_transport_send_response(vchild, skb);
 
-	release_sock(child);
-
 	sk->sk_data_ready(sk);
 	return 0;
 }
-- 
2.43.0

[syzbot ci] Re: vsock/virtio: fix memory leak in virtio_transport_recv_listen()

[syzbot ci]

syzbot ci has tested the following series

[v1] vsock/virtio: fix memory leak in virtio_transport_recv_listen()
https://lore.kernel.org/all/20260424150310.57228-1-kartikey406@gmail.com
* [PATCH] vsock/virtio: fix memory leak in virtio_transport_recv_listen()

and found the following issue:
possible deadlock in virtio_transport_recv_listen

Full report is available here:
https://ci.syzbot.org/series/e5f3f5f1-8e83-492f-833b-dc526b61d0ef

***

possible deadlock in virtio_transport_recv_listen

tree:      net-next
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net-next.git
base:      e728258debd553c95d2e70f9cd97c9fde27c7130
arch:      amd64
compiler:  Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
config:    https://ci.syzbot.org/builds/bcc67f3f-b441-4d11-8573-f26ca268da65/config
syz repro: https://ci.syzbot.org/findings/fce9fa9b-692a-420c-956f-12c7e6d75e2e/syz_repro

============================================
WARNING: possible recursive locking detected
syzkaller #0 Not tainted
--------------------------------------------
kworker/1:5/6007 is trying to acquire lock:
ffff88817492e2a0 (sk_lock-AF_VSOCK){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1713 [inline]
ffff88817492e2a0 (sk_lock-AF_VSOCK){+.+.}-{0:0}, at: virtio_transport_recv_listen+0x13a5/0x2230 net/vmw_vsock/virtio_transport_common.c:1593

but task is already holding lock:
ffff88817492e2a0 (sk_lock-AF_VSOCK){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1713 [inline]
ffff88817492e2a0 (sk_lock-AF_VSOCK){+.+.}-{0:0}, at: virtio_transport_recv_pkt+0xfde/0x2bb0 net/vmw_vsock/virtio_transport_common.c:1671

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(sk_lock-AF_VSOCK);
  lock(sk_lock-AF_VSOCK);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

3 locks held by kworker/1:5/6007:
 #0: ffff888112391940 ((wq_completion)vsock-loopback){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3277 [inline]
 #0: ffff888112391940 ((wq_completion)vsock-loopback){+.+.}-{0:0}, at: process_scheduled_works+0xa35/0x1860 kernel/workqueue.c:3385
 #1: ffffc900030dfc40 ((work_completion)(&vsock->pkt_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3278 [inline]
 #1: ffffc900030dfc40 ((work_completion)(&vsock->pkt_work)){+.+.}-{0:0}, at: process_scheduled_works+0xa70/0x1860 kernel/workqueue.c:3385
 #2: ffff88817492e2a0 (sk_lock-AF_VSOCK){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1713 [inline]
 #2: ffff88817492e2a0 (sk_lock-AF_VSOCK){+.+.}-{0:0}, at: virtio_transport_recv_pkt+0xfde/0x2bb0 net/vmw_vsock/virtio_transport_common.c:1671

stack backtrace:
CPU: 1 UID: 0 PID: 6007 Comm: kworker/1:5 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Workqueue: vsock-loopback vsock_loopback_work
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
 print_deadlock_bug+0x279/0x290 kernel/locking/lockdep.c:3041
 check_deadlock kernel/locking/lockdep.c:3093 [inline]
 validate_chain kernel/locking/lockdep.c:3895 [inline]
 __lock_acquire+0x253f/0x2cf0 kernel/locking/lockdep.c:5237
 lock_acquire+0x106/0x350 kernel/locking/lockdep.c:5868
 lock_sock_nested+0x41/0x100 net/core/sock.c:3783
 lock_sock include/net/sock.h:1713 [inline]
 virtio_transport_recv_listen+0x13a5/0x2230 net/vmw_vsock/virtio_transport_common.c:1593
 virtio_transport_recv_pkt+0x174b/0x2bb0 net/vmw_vsock/virtio_transport_common.c:1695
 vsock_loopback_work+0x32c/0x3f0 net/vmw_vsock/vsock_loopback.c:142
 process_one_work kernel/workqueue.c:3302 [inline]
 process_scheduled_works+0xb5d/0x1860 kernel/workqueue.c:3385
 worker_thread+0xa53/0xfc0 kernel/workqueue.c:3466
 kthread+0x388/0x470 kernel/kthread.c:436
 ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
 </TASK>


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a patch for this bug, please reply with `#syz test`
(should be on a separate line).

The patch should be attached to the email.
Note: arguments like custom git repos and branches are not supported.

Re: [PATCH] vsock/virtio: fix memory leak in virtio_transport_recv_listen()

[Stefano Garzarella]

On Fri, Apr 24, 2026 at 08:33:10PM +0530, Deepanshu Kartikey wrote:
>Two bugs exist in virtio_transport_recv_listen():

Two bugs, two fixes, two patches usually.

>
>1. On the transport assignment error path, sk_acceptq_added() is called
>   but sk_acceptq_removed() is never called when vsock_assign_transport()
>   fails or assigns a different transport than expected. This causes the
>   parent listener's accept backlog counter to be permanently inflated,
>   eventually causing sk_acceptq_is_full() to reject legitimate incoming
>   connections.

Wait, I can't see this issue. sk_acceptq_added() is called after the
vsock_assign_transport(), so why we should call sk_acceptq_removed()
in the error path of vsock_assign_transport()?

Maybe I'm missing something.

>
>2. There is a race between __vsock_release() and vsock_enqueue_accept().
>   __vsock_release() sets sk->sk_shutdown to SHUTDOWN_MASK and flushes
>   the accept queue under the parent socket lock. However,
>   virtio_transport_recv_listen() checks sk_shutdown and subsequently
>   calls vsock_enqueue_accept() without holding the parent socket lock.

Are you sure about this?

virtio_transport_recv_listen() is called only by 
virtio_transport_recv_pkt() after calling lock_sock(sk), so I'm really 
confused.

>   This means a child socket can be enqueued after __vsock_release() has
>   already flushed the queue, causing the child socket and its associated
>   resources to leak
>   permanently. The existing comment in the code hints at this race but
>   the fix was never implemented.

Are you referring to:
	/* __vsock_release() might have already flushed accept_queue.
	 * Subsequent enqueues would lead to a memory leak.
	 */
	if (sk->sk_shutdown == SHUTDOWN_MASK) {
		virtio_transport_reset_no_sock(t, skb, sock_net(sk));
		return -ESHUTDOWN;
	}

In this case I think we are saying that we are doing this check exactly 
to avoid that issue.

>
>Fix both issues: add sk_acceptq_removed() on the transport error path,

Again, better to fix the 2 issues with 2 patches (same series is fine).

>and re-check sk->sk_shutdown under the parent socket lock before calling
>vsock_enqueue_accept() to close the race window. The child socket lock
>is released before acquiring the parent socket lock to maintain correct
>lock ordering (parent before child).
>

We are missing the Fixes tag, and I think we can target the `net` tree
with this patch (i.e. [PATCH net]), see:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html

>Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
>Tested-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 416d533f493d..fad5fa4a4296 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1578,6 +1578,7 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> 	 */
> 	if (ret || vchild->transport != &t->transport) {
> 		release_sock(child);
>+		sk_acceptq_removed(sk);

Ditto, are we sure about this?

> 		virtio_transport_reset_no_sock(t, skb, sock_net(sk));
> 		sock_put(child);
> 		return ret;
>@@ -1588,11 +1589,19 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> 		child->sk_write_space(child);
>
> 	vsock_insert_connected(vchild);
>+	release_sock(child);
>+	lock_sock(sk);

IMO this is a deadlock with the lock_sock(sk) called by the caller.

Also a comment would be helpful here to explain why we're doing this.

>+	if (sk->sk_shutdown == SHUTDOWN_MASK) {
>+		release_sock(sk);
>+		sk_acceptq_removed(sk);
>+		virtio_transport_reset_no_sock(t, skb, sock_net(sk));
>+		sock_put(child);
>+		return -ESHUTDOWN;

Since this is very similar to the error path of 
vsock_assign_transport(), I think it would be better to start by 
defining a common error path for the function and use labels to exit, so 
we can avoid duplicating the code multiple times.

>+	}
> 	vsock_enqueue_accept(sk, child);
>+	release_sock(sk);
> 	virtio_transport_send_response(vchild, skb);
>
>-	release_sock(child);
>-

TBH I'm really worried about this patch since both fixes are completely 
wrong IMO.

Thanks,
Stefano

> 	sk->sk_data_ready(sk);
> 	return 0;
> }
>-- 
>2.43.0
>

Re: [PATCH] vsock/virtio: fix memory leak in virtio_transport_recv_listen()

[Deepanshu Kartikey]

On Mon, Apr 27, 2026 at 7:15 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Fri, Apr 24, 2026 at 08:33:10PM +0530, Deepanshu Kartikey wrote:
> >Two bugs exist in virtio_transport_recv_listen():
>
> Two bugs, two fixes, two patches usually.
>
> >
> >1. On the transport assignment error path, sk_acceptq_added() is called
> >   but sk_acceptq_removed() is never called when vsock_assign_transport()
> >   fails or assigns a different transport than expected. This causes the
> >   parent listener's accept backlog counter to be permanently inflated,
> >   eventually causing sk_acceptq_is_full() to reject legitimate incoming
> >   connections.
>
> Wait, I can't see this issue. sk_acceptq_added() is called after the
> vsock_assign_transport(), so why we should call sk_acceptq_removed()
> in the error path of vsock_assign_transport()?
>
> Maybe I'm missing something.
>
> >
> >2. There is a race between __vsock_release() and vsock_enqueue_accept().
> >   __vsock_release() sets sk->sk_shutdown to SHUTDOWN_MASK and flushes
> >   the accept queue under the parent socket lock. However,
> >   virtio_transport_recv_listen() checks sk_shutdown and subsequently
> >   calls vsock_enqueue_accept() without holding the parent socket lock.
>
> Are you sure about this?
>
> virtio_transport_recv_listen() is called only by
> virtio_transport_recv_pkt() after calling lock_sock(sk), so I'm really
> confused.
>
> >   This means a child socket can be enqueued after __vsock_release() has
> >   already flushed the queue, causing the child socket and its associated
> >   resources to leak
> >   permanently. The existing comment in the code hints at this race but
> >   the fix was never implemented.
>
> Are you referring to:
>         /* __vsock_release() might have already flushed accept_queue.
>          * Subsequent enqueues would lead to a memory leak.
>          */
>         if (sk->sk_shutdown == SHUTDOWN_MASK) {
>                 virtio_transport_reset_no_sock(t, skb, sock_net(sk));
>                 return -ESHUTDOWN;
>         }
>
> In this case I think we are saying that we are doing this check exactly
> to avoid that issue.
>
> >
> >Fix both issues: add sk_acceptq_removed() on the transport error path,
>
> Again, better to fix the 2 issues with 2 patches (same series is fine).
>
> >and re-check sk->sk_shutdown under the parent socket lock before calling
> >vsock_enqueue_accept() to close the race window. The child socket lock
> >is released before acquiring the parent socket lock to maintain correct
> >lock ordering (parent before child).
> >
>
> We are missing the Fixes tag, and I think we can target the `net` tree
> with this patch (i.e. [PATCH net]), see:
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
>
> >Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
> >Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
> >Tested-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
> >Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> >---
> > net/vmw_vsock/virtio_transport_common.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >index 416d533f493d..fad5fa4a4296 100644
> >--- a/net/vmw_vsock/virtio_transport_common.c
> >+++ b/net/vmw_vsock/virtio_transport_common.c
> >@@ -1578,6 +1578,7 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> >        */
> >       if (ret || vchild->transport != &t->transport) {
> >               release_sock(child);
> >+              sk_acceptq_removed(sk);
>
> Ditto, are we sure about this?
>
> >               virtio_transport_reset_no_sock(t, skb, sock_net(sk));
> >               sock_put(child);
> >               return ret;
> >@@ -1588,11 +1589,19 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> >               child->sk_write_space(child);
> >
> >       vsock_insert_connected(vchild);
> >+      release_sock(child);
> >+      lock_sock(sk);
>
> IMO this is a deadlock with the lock_sock(sk) called by the caller.
>
> Also a comment would be helpful here to explain why we're doing this.
>
> >+      if (sk->sk_shutdown == SHUTDOWN_MASK) {
> >+              release_sock(sk);
> >+              sk_acceptq_removed(sk);
> >+              virtio_transport_reset_no_sock(t, skb, sock_net(sk));
> >+              sock_put(child);
> >+              return -ESHUTDOWN;
>
> Since this is very similar to the error path of
> vsock_assign_transport(), I think it would be better to start by
> defining a common error path for the function and use labels to exit, so
> we can avoid duplicating the code multiple times.
>
> >+      }
> >       vsock_enqueue_accept(sk, child);
> >+      release_sock(sk);
> >       virtio_transport_send_response(vchild, skb);
> >
> >-      release_sock(child);
> >-
>
> TBH I'm really worried about this patch since both fixes are completely
> wrong IMO.
>
> Thanks,
> Stefano
>
> >       sk->sk_data_ready(sk);
> >       return 0;
> > }
> >--
> >2.43.0
> >
>


Hi Stefano,

Thank you for the detailed review!

You are correct on both points. I apologize for the confusion — I was
looking at an older version of the code where sk_acceptq_added() was
called BEFORE vsock_assign_transport(), which made the
sk_acceptq_removed() fix appear necessary. In the current kernel,
sk_acceptq_added() is already moved to after vsock_assign_transport(),
so that issue no longer exists.

Regarding the lock_sock(sk) fix — you are also correct that
virtio_transport_recv_pkt() already holds lock_sock(sk) before calling
virtio_transport_recv_listen(), so our second fix would indeed cause a
deadlock. I missed that completely.

I am still investigating the root cause of the memory leak reported by
syzbot. The backtrace points to the vsock loopback path
(vsock_loopback_work), so I am looking there next. I will send a v2
once I have a correct analysis and fix.

Sorry again for the noise.

Thanks,
Deeranshu Kartikey

Re: [PATCH] vsock/virtio: fix memory leak in virtio_transport_recv_listen()

[Stefano Garzarella]

On Tue, Apr 28, 2026 at 01:54:36PM +0530, Deepanshu Kartikey wrote:
>On Mon, Apr 27, 2026 at 7:15 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>>
>> On Fri, Apr 24, 2026 at 08:33:10PM +0530, Deepanshu Kartikey wrote:
>> >Two bugs exist in virtio_transport_recv_listen():
>>
>> Two bugs, two fixes, two patches usually.
>>
>> >
>> >1. On the transport assignment error path, sk_acceptq_added() is called
>> >   but sk_acceptq_removed() is never called when vsock_assign_transport()
>> >   fails or assigns a different transport than expected. This causes the
>> >   parent listener's accept backlog counter to be permanently inflated,
>> >   eventually causing sk_acceptq_is_full() to reject legitimate incoming
>> >   connections.
>>
>> Wait, I can't see this issue. sk_acceptq_added() is called after the
>> vsock_assign_transport(), so why we should call sk_acceptq_removed()
>> in the error path of vsock_assign_transport()?
>>
>> Maybe I'm missing something.
>>
>> >
>> >2. There is a race between __vsock_release() and vsock_enqueue_accept().
>> >   __vsock_release() sets sk->sk_shutdown to SHUTDOWN_MASK and flushes
>> >   the accept queue under the parent socket lock. However,
>> >   virtio_transport_recv_listen() checks sk_shutdown and subsequently
>> >   calls vsock_enqueue_accept() without holding the parent socket lock.
>>
>> Are you sure about this?
>>
>> virtio_transport_recv_listen() is called only by
>> virtio_transport_recv_pkt() after calling lock_sock(sk), so I'm really
>> confused.
>>
>> >   This means a child socket can be enqueued after __vsock_release() has
>> >   already flushed the queue, causing the child socket and its associated
>> >   resources to leak
>> >   permanently. The existing comment in the code hints at this race but
>> >   the fix was never implemented.
>>
>> Are you referring to:
>>         /* __vsock_release() might have already flushed accept_queue.
>>          * Subsequent enqueues would lead to a memory leak.
>>          */
>>         if (sk->sk_shutdown == SHUTDOWN_MASK) {
>>                 virtio_transport_reset_no_sock(t, skb, sock_net(sk));
>>                 return -ESHUTDOWN;
>>         }
>>
>> In this case I think we are saying that we are doing this check exactly
>> to avoid that issue.
>>
>> >
>> >Fix both issues: add sk_acceptq_removed() on the transport error path,
>>
>> Again, better to fix the 2 issues with 2 patches (same series is fine).
>>
>> >and re-check sk->sk_shutdown under the parent socket lock before calling
>> >vsock_enqueue_accept() to close the race window. The child socket lock
>> >is released before acquiring the parent socket lock to maintain correct
>> >lock ordering (parent before child).
>> >
>>
>> We are missing the Fixes tag, and I think we can target the `net` tree
>> with this patch (i.e. [PATCH net]), see:
>> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
>>
>> >Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>> >Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
>> >Tested-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>> >Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
>> >---
>> > net/vmw_vsock/virtio_transport_common.c | 13 +++++++++++--
>> > 1 file changed, 11 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> >index 416d533f493d..fad5fa4a4296 100644
>> >--- a/net/vmw_vsock/virtio_transport_common.c
>> >+++ b/net/vmw_vsock/virtio_transport_common.c
>> >@@ -1578,6 +1578,7 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
>> >        */
>> >       if (ret || vchild->transport != &t->transport) {
>> >               release_sock(child);
>> >+              sk_acceptq_removed(sk);
>>
>> Ditto, are we sure about this?
>>
>> >               virtio_transport_reset_no_sock(t, skb, sock_net(sk));
>> >               sock_put(child);
>> >               return ret;
>> >@@ -1588,11 +1589,19 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
>> >               child->sk_write_space(child);
>> >
>> >       vsock_insert_connected(vchild);
>> >+      release_sock(child);
>> >+      lock_sock(sk);
>>
>> IMO this is a deadlock with the lock_sock(sk) called by the caller.
>>
>> Also a comment would be helpful here to explain why we're doing this.
>>
>> >+      if (sk->sk_shutdown == SHUTDOWN_MASK) {
>> >+              release_sock(sk);
>> >+              sk_acceptq_removed(sk);
>> >+              virtio_transport_reset_no_sock(t, skb, sock_net(sk));
>> >+              sock_put(child);
>> >+              return -ESHUTDOWN;
>>
>> Since this is very similar to the error path of
>> vsock_assign_transport(), I think it would be better to start by
>> defining a common error path for the function and use labels to exit, so
>> we can avoid duplicating the code multiple times.
>>
>> >+      }
>> >       vsock_enqueue_accept(sk, child);
>> >+      release_sock(sk);
>> >       virtio_transport_send_response(vchild, skb);
>> >
>> >-      release_sock(child);
>> >-
>>
>> TBH I'm really worried about this patch since both fixes are completely
>> wrong IMO.
>>
>> Thanks,
>> Stefano
>>
>> >       sk->sk_data_ready(sk);
>> >       return 0;
>> > }
>> >--
>> >2.43.0
>> >
>>
>
>
>Hi Stefano,
>
>Thank you for the detailed review!
>
>You are correct on both points. I apologize for the confusion — I was
>looking at an older version of the code where sk_acceptq_added() was
>called BEFORE vsock_assign_transport(), which made the
>sk_acceptq_removed() fix appear necessary. In the current kernel,
>sk_acceptq_added() is already moved to after vsock_assign_transport(),
>so that issue no longer exists.
>
>Regarding the lock_sock(sk) fix — you are also correct that
>virtio_transport_recv_pkt() already holds lock_sock(sk) before calling
>virtio_transport_recv_listen(), so our second fix would indeed cause a
>deadlock. I missed that completely.
>
>I am still investigating the root cause of the memory leak reported by
>syzbot. The backtrace points to the vsock loopback path
>(vsock_loopback_work), so I am looking there next. I will send a v2
>once I have a correct analysis and fix.

Okay, thanks for looking into that issue, feel free to chat here, or in 
reply to the syzbot report if you have some new findings.

Thanks,
Stefano