Re: socket state questions

On Mon, Apr 27, 2026 at 2:41 PM Mark Johnston <[email protected]> wrote:
>
> 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.
Interesting. First off, pru_getattr() didn't exist when he did this commit.
There is pru_getpeeraddr() and pru_getsockaddr(), which for TCP
are set to in_getpeeraddr() and in_getsockaddr().
But neither of them ever return an error.
In main, they are sopeeraddr() and sosockaddr(), but they
still call in_getpeeraddr() and in_getsockaddr(), which never
return an error.

So, something else must have been causing the failure.
Maybe the old code caused sosetopt() to fail, due to a
flag like INP_DROPPED or INP_TIMEWAIT being set?
The new code for tcp_ctloutput() is quite different, using
stuff like t_fb which I have never heard of?

I wonder if the svc_vc_create() code should just check for
SS_DISCONNECTED and return an error for that case
or can it trust sosetopt() to fail?

rick

>
> On a somewhat related note, should svc_vc_create() be checking for
> errors from solisten()?
>
> > Thanks for any help with this, rick