Re: socket state questions

On Sun, Apr 26, 2026 at 03:24:31PM -0700, Rick Macklem wrote:
> Hi,
> 
> I'm trying to figure out what might be causing the
> crashes reported by bugzilla PR#293127.
> 
> When I look in svc_vc.c I find this:
> svc_vc_create(SVCPOOL *pool, struct socket *so, size_t sendsize,
>     size_t recvsize)
> {
>    SVCXPRT *xprt;
>    int error;
> 
>    SOCK_LOCK(so);
>    if (so->so_state & (SS_ISCONNECTED|SS_ISDISCONNECTED)) {
> 
> This "if" is testing to see if the socket should be used for listening or
> if it one returned from "accept". For NFS, this should always be true.
> 
> I'm wondering about a couple of things...
> - Should SS_ISDISCONNECTING be here, as well, or will accepted
>   TCP sockets go directly from SS_ISCONNECTED to SS_ISDISCONNECTED?

I don't think so: it's possible for TCP sockets to go through
SS_ISDISCONNECTING, but only via system calls, and soisdisconnecting()
doesn't clear SS_ISCONNECTED.

> - Why would SS_ISDISCONNECTED be here? Can a socket go from
>   SS_ISDISCONNECTED back to SS_ISCONNECTED?
>   If not, the only reason I can think of is so that it will get closed by
>   svc_vc_destroy_common(), but then why not just soclose() it here?

It was added there explicitly in commit dad14216507bf:

commit dad14216507bfff12693742399c3540722178263
Author: John Baldwin <[email protected]>
Date:   Mon Apr 8 19:03:01 2013 +0000

    Fix a potential socket leak in the NFS server.  If a client closes its
    connection after it was accepted by the userland nfsd process but before
    it was handled off to svc_vc_create() in the kernel, then svc_vc_create()
    would see it as a new listen socket and try to listen on it leaving a
    dangling reference to the socket.  Instead, check for disconnected sockets
    and treat them like a connected socket.  The call to pru_getaddr() should
    fail and cause svc_vc_create() to fail.  Note that we need to lock the
    socket to get a consistent snapshot of so_state since there is a window
    in soisdisconnected() where both flags are clear.

On a somewhat related note, should svc_vc_create() be checking for
errors from solisten()?

> Thanks for any help with this, rick