Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout

[Stefano Garzarella <sgarzare@redhat.com>]

On Tue, Jul 21, 2026 at 01:34:03AM +0800, Phi Nguyen wrote:
>On 7/20/2026 4:17 PM, Stefano Garzarella wrote:
>>On Mon, Jul 20, 2026 at 05:57:47AM +0800, Nguyen Dinh Phi wrote:
>>>After vsock_connect() exits the wait loop due to sk->sk_err being
>>>set, the error was read but not cleared. This left sk->sk_err set
>>>for subsequent operations.
>>
>>So, is this a fix? If yes, we should put a Fixes tag.
>>
>>Also, can you describe how to trigger the issue?
>>
>>Because I see this in vsock_connect(), so I thought it was in some 
>>way already handled:
>>
>>         /* sk_err might have been set as a result of an earlier
>>          * (failed) connect attempt.
>>          */
>>         sk->sk_err = 0;
>>
>This only handles the case where the function following the failed 
>connect is another connect() call.

So, can we remove that with this patch, or better to leave as defensive
action?

>
>>>Switch to sock_error() which atomically reads and clears sk->sk_err,
>>>so the error is consumed when returned.
>>>
>>>Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
>>>Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>>
>>Can you explain how this patch fixes that issue?
>>(this should be the first information to be put in the commit message IMHO)
>>
>>I'd like to understand better if this is a fix of real bug or just 
>>an improvement to the code (which is fine by me).
>>
>>Thanks,
>>Stefano
>>
>Here are the steps of the syzkaller reproducer:
>
>  r0 = socket(AF_VSOCK, SOCK_STREAM, 0)
>
>  bind(r0, {VMADDR_CID_ANY, PORT})
>
>  connect(r0, {VMADDR_CID_LOCAL, PORT})
>
>  listen(r0, backlog)
>
>  r1 = socket(AF_VSOCK, SOCK_STREAM, 0)
>
>  connect(r1, {VMADDR_CID_LOCAL, PORT})
>
>  connect(r0 -> self) -> -1, EPROTO
>
>  listen(r0)          -> 0
>
>  connect(r1 -> r0)   -> 0
>
>  accept(r0)          -> -1, EPROTO
>
>Basically, it creates a socket (r0) and triggers a self-connect after 
>binding it. This self-connect fails with EPROTO because it loops back 
>to r0 while the socket is still in the TCP_SYN_SENT state, causing it 
>to be incorrectly dispatched to the connecting-client path. The 
>unexpected packet type encountered there sets sk_err to EPROTO.
>
>After that, it invokes a listen() call on the same socket. This 
>listen() call succeeds because the kernel's listening path never 
>inspects or clears sk_err. Then, a new socket (r1) is created as a 
>normal client and connects to r0. However, vsock_accept() rejects this 
>incoming connection because the listener's sk_err still holds the 
>EPROTO error from the earlier failed self-connect.
>
>This rejection causes the child socket created for r1's connection to 
>never be freed on virtio or hyperv transports; only the VMCI transport 
>implements pending_work to revisit and clean up a rejected socket
>This patch will prevent the rejection branch to occur in this scenario.

Okay, get it now, thanks! Please include a summary of this in the commit 
description.

I understand that this resolves syzbot's specific test case, but it 
would be best to handle rejected sockets more effectively in af_vsock.c 
rather than in the transport layers (if possible). In any case, this can 
be done in another patch.

>
>I think we might schedule the cleanup worker to run in the rejection 
>path for these transports as well.

Yeah, we need to handle that part better, I think it's a leftover when 
we generalized AF_VSOCK to support more transport than vmci.

Indeed this part is a bit confusing:

		/* If the listener socket has received an error, then we should
		 * reject this socket and return.  Note that we simply mark the
		 * socket rejected, drop our reference, and let the cleanup
		 * function handle the cleanup; the fact that we found it in
		 * the listener's accept queue guarantees that the cleanup
		 * function hasn't run yet.
		 */
		if (err) {
			vconnected->rejected = true;
		} else {


Would be nice to handle everything in af_vsock.c in some way.

In conclusion, the patch LGTM, but please expand the commit description, 
add the Fixes tag, and target the net tree in the v2.

Thanks,
Stefano