On Mon, Jun 04, 2018 at 03:25:17PM +0200, Martin Pieuchot wrote:
> For pfkey and routing sockets, calling sofree() in pr_detach is a noop.
> The reason is that `SS_NOFDREF' hasn't been set at this point so the
> socket won't be freed.
>
> So I'd like to remove the sofree() and add an assert instead. sofree()
> will need to change soon to be able to deal with per-socket locks as
> well as global locks. So having fewer of them help.
>
> ok?
I'm not sure if I like it when implementations differ in use of the socket
functions. In general I would prefer if the socket interface is used the
same way by all protocols so that reviewing them is simpler.
I understand that the mentioned ones are already kind of special
snowflakes so maybe this is not an issue.
> Index: kern/uipc_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.221
> diff -u -p -r1.221 uipc_socket.c
> --- kern/uipc_socket.c 8 May 2018 15:03:27 -0000 1.221
> +++ kern/uipc_socket.c 4 Jun 2018 10:16:30 -0000
> @@ -282,8 +282,7 @@ drop:
> error = error2;
> }
> discard:
> - if (so->so_state & SS_NOFDREF)
> - panic("soclose NOFDREF: so %p, so_type %d", so, so->so_type);
> + KASSERT((so->so_state & SS_NOFDREF) == 0);
> so->so_state |= SS_NOFDREF;
> sofree(so);
> sounlock(s);
> @@ -306,8 +305,7 @@ soaccept(struct socket *so, struct mbuf
>
> soassertlocked(so);
>
> - if ((so->so_state & SS_NOFDREF) == 0)
> - panic("soaccept !NOFDREF: so %p, so_type %d", so, so->so_type);
> + KASSERT((so->so_state & SS_NOFDREF) != 0);
> so->so_state &= ~SS_NOFDREF;
> if ((so->so_state & SS_ISDISCONNECTED) == 0 ||
> (so->so_proto->pr_flags & PR_ABRTACPTDIS) == 0)
> Index: net/pfkeyv2.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.180
> diff -u -p -r1.180 pfkeyv2.c
> --- net/pfkeyv2.c 19 May 2018 20:04:55 -0000 1.180
> +++ net/pfkeyv2.c 4 Jun 2018 10:17:11 -0000
> @@ -323,8 +323,9 @@ pfkeyv2_detach(struct socket *so)
> refcnt_finalize(&kp->refcnt, "pfkeyrefs");
>
> so->so_pcb = NULL;
> - sofree(so);
> + KASSERT((so->so_state & SS_NOFDREF) == 0);
> free(kp, M_PCB, sizeof(struct keycb));
> +
> return (0);
> }
>
> Index: net/rtsock.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.265
> diff -u -p -r1.265 rtsock.c
> --- net/rtsock.c 14 May 2018 07:33:59 -0000 1.265
> +++ net/rtsock.c 4 Jun 2018 10:17:10 -0000
> @@ -296,7 +296,7 @@ route_detach(struct socket *so)
> refcnt_finalize(&rop->refcnt, "rtsockrefs");
>
> so->so_pcb = NULL;
> - sofree(so);
> + KASSERT((so->so_state & SS_NOFDREF) == 0);
> free(rop, M_PCB, sizeof(struct routecb));
>
> return (0);
>
--
:wq Claudio
