On 23/7/26 16:24, Stefano Garzarella wrote:
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?
I think connect() already delivered the error directly to the
caller when it returned -- there's no reason for poll() to keep
reporting that same, already-handled error afterward.
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.
yes, __inet_stream_connect() does it, and actually, other protocols like
Bluetooth, TIPC, x25... are also consume sk_err with sock_error() after
their wait loop.
I think it is an established pattern, so I'd rather keep sock_error() in
vsock_connect() than move it to vsock_listen(). vsock_listen() would
only protect the listen() path; I see other places in af_vsock.c use
this sk_err.
...
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 ?
I couldn't find it for listener side too.
Thanks,
Phi.
