Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
Stefano Garzarella <[email protected]> Thu, 23 Jul 2026 10:24:05 +0200
| Newsgroups | dev.linux.lists.virtualization,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <amHN9mOGj7oMak4-@sgarzare-redhat> |
On Thu, Jul 23, 2026 at 08:00:31AM +0200, Michal Luczaj wrote: >On 7/23/26 06:14, Nguyen Dinh Phi [SG] wrote: >> On 22/7/26 15:55, Stefano Garzarella wrote: >>> 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? >>> >> >> I prefer to keep it here as defensive action > >Is changing how vsock_poll() behaves intended? Good point, but IIUC __inet_stream_connect() is also using consuming the error with sock_error(). > >I.e. if sk_err should be kept after a failed connect(), what about >`sk->sk_err = 0;` in vsock_listen() instead? Yeah, maybe this is a bit less invasive. > >>> ... >>> 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. > >Speaking of leftovers, I have trouble understanding where does vsock set >sk_err on listener sockets anyway. If it doesn't, why vsock_accept() checks >for it? I can't also see where it can be set TBH. Should we remove it ? Thanks, Stefano