Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
"Nguyen Dinh Phi [SG]" <[email protected]> Wed, 29 Jul 2026 17:46:29 +0800
| Newsgroups | dev.linux.lists.virtualization,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On 28/7/26 16:21, Stefano Garzarella wrote: > On Fri, Jul 24, 2026 at 03:34:23PM +0800, Nguyen Dinh Phi [SG] wrote: >> On 24/7/26 05:43, Michal Luczaj wrote: >>> On 7/23/26 12:26, Nguyen Dinh Phi [SG] wrote: >>>>>>>> ... >>>>>>>> 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. >>> >>> Removing sk_err handling from vsock_accept() solves the problem, right? >>> >>> thanks, >>> Michal >> >> Yes, confirmed, removing sk_err checks from vsock_accept() does solve >> the problem. > > Okay, so maybe better on going on this direction. WDYT? > > Stefano > I'm still a bit concerned about how connect() and poll() interact here, even with the sk_err checks removed from vsock_accept(). For example: vsock_accept() now lets us reuse a socket whose connect() failed (call it r0) as syzbot reproducer does. After listen(), r0 becomes a listener (sk_state == TCP_LISTEN) and works correctly -- it accepts connections. But poll() on r0 still marks POLLERR, even though there is no error on that socket at that point. As I understand it, sk_err holds an error that has not yet been reported to userspace. In the blocking vsock_connect() case we have already read that error and returned it to the caller, so it is no longer pending Shouldn't sk_err be consumed/cleared when vsock_connect() returns it to userspace? If you think the POLLERR above is acceptable, I'm fine going with just the vsock_accept() change. Thanks, Phi.