svn commit: r303855 - in head/sys: kern sys

[Mark Johnston]

On Tue, Aug 09, 2016 at 12:53:47AM +0200, Oliver Pinter wrote:
> Hi!
> 
> Can you please MFC back this change 10-STABLE together with the
> following: https://github.com/HardenedBSD/hardenedBSD/commit/576619e564618bca3675db57580d8e1f76bd2ac7
> 
> This issue is still exists on 10-STABLE, as you can test with the
> linked program from phabricator:
> https://people.freebsd.org/~mjg/reproducers/unp-gc-panic.c

Hm, I don't think this could be MFCed directly. It changes the kernel
ABI by modifying the argument of dom_dispose(). This could be fixed in
stable/10 with a hack to call the unix domain socket code directly when
appropriate, which I think is preferable to the current state of things.
I'll look into it further.

> 
> On Mon, Aug 8, 2016 at 10:25 PM, Mark Johnston <markj at freebsd.org> wrote:
> > Author: markj
> > Date: Mon Aug  8 20:25:04 2016
> > New Revision: 303855
> > URL: https://svnweb.freebsd.org/changeset/base/303855
> >
> > Log:
> >   Handle races with listening socket close when connecting a unix socket.
> >
> >   If the listening socket is closed while sonewconn() is executing, the
> >   nascent child socket is aborted, which results in recursion on the
> >   unp_link lock when the child's pru_detach method is invoked. Fix this
> >   by using a flag to mark such sockets, and skip a part of the socket's
> >   teardown during detach.
> >
> >   Reported by:  Raviprakash Darbha <rdarbha at juniper.net>
> >   Tested by:    pho
> >   MFC after:    2 weeks
> >   Differential Revision:        https://reviews.freebsd.org/D7398
> >
> > Modified:
> >   head/sys/kern/uipc_usrreq.c
> >   head/sys/sys/unpcb.h
> >
> > Modified: head/sys/kern/uipc_usrreq.c
> > ==============================================================================
> > --- head/sys/kern/uipc_usrreq.c Mon Aug  8 20:23:11 2016        (r303854)
> > +++ head/sys/kern/uipc_usrreq.c Mon Aug  8 20:25:04 2016        (r303855)
> > @@ -430,6 +430,8 @@ uipc_attach(struct socket *so, int proto
> >         unp->unp_socket = so;
> >         so->so_pcb = unp;
> >         unp->unp_refcount = 1;
> > +       if (so->so_head != NULL)
> > +               unp->unp_flags |= UNP_NASCENT;
> >
> >         UNP_LIST_LOCK();
> >         unp->unp_gencnt = ++unp_gencnt;
> > @@ -652,14 +654,22 @@ uipc_detach(struct socket *so)
> >         unp = sotounpcb(so);
> >         KASSERT(unp != NULL, ("uipc_detach: unp == NULL"));
> >
> > -       UNP_LINK_WLOCK();
> > +       vp = NULL;
> > +       local_unp_rights = 0;
> > +
> >         UNP_LIST_LOCK();
> > -       UNP_PCB_LOCK(unp);
> >         LIST_REMOVE(unp, unp_link);
> >         unp->unp_gencnt = ++unp_gencnt;
> >         --unp_count;
> >         UNP_LIST_UNLOCK();
> >
> > +       if ((unp->unp_flags & UNP_NASCENT) != 0) {
> > +               UNP_PCB_LOCK(unp);
> > +               goto teardown;
> > +       }
> > +       UNP_LINK_WLOCK();
> > +       UNP_PCB_LOCK(unp);
> > +
> >         /*
> >          * XXXRW: Should assert vp->v_socket == so.
> >          */
> > @@ -687,6 +697,7 @@ uipc_detach(struct socket *so)
> >         }
> >         local_unp_rights = unp_rights;
> >         UNP_LINK_WUNLOCK();
> > +teardown:
> >         unp->unp_socket->so_pcb = NULL;
> >         saved_unp_addr = unp->unp_addr;
> >         unp->unp_addr = NULL;
> > @@ -1473,6 +1484,7 @@ unp_connect2(struct socket *so, struct s
> >
> >         if (so2->so_type != so->so_type)
> >                 return (EPROTOTYPE);
> > +       unp2->unp_flags &= ~UNP_NASCENT;
> >         unp->unp_conn = unp2;
> >
> >         switch (so->so_type) {
> >
> > Modified: head/sys/sys/unpcb.h
> > ==============================================================================
> > --- head/sys/sys/unpcb.h        Mon Aug  8 20:23:11 2016        (r303854)
> > +++ head/sys/sys/unpcb.h        Mon Aug  8 20:25:04 2016        (r303855)
> > @@ -103,11 +103,6 @@ struct unpcb {
> >  #define        UNP_WANTCRED                    0x004   /* credentials wanted */
> >  #define        UNP_CONNWAIT                    0x008   /* connect blocks until accepted */
> >
> > -#define        UNPGC_REF                       0x1     /* unpcb has external ref. */
> > -#define        UNPGC_DEAD                      0x2     /* unpcb might be dead. */
> > -#define        UNPGC_SCANNED                   0x4     /* Has been scanned. */
> > -#define        UNPGC_IGNORE_RIGHTS             0x8     /* Attached rights are freed */
> > -
> >  /*
> >   * These flags are used to handle non-atomicity in connect() and bind()
> >   * operations on a socket: in particular, to avoid races between multiple
> > @@ -115,6 +110,15 @@ struct unpcb {
> >   */
> >  #define        UNP_CONNECTING                  0x010   /* Currently connecting. */
> >  #define        UNP_BINDING                     0x020   /* Currently binding. */
> > +#define        UNP_NASCENT                     0x040   /* Newborn child socket. */
> > +
> > +/*
> > + * Flags in unp_gcflag.
> > + */
> > +#define        UNPGC_REF                       0x1     /* unpcb has external ref. */
> > +#define        UNPGC_DEAD                      0x2     /* unpcb might be dead. */
> > +#define        UNPGC_SCANNED                   0x4     /* Has been scanned. */
> > +#define        UNPGC_IGNORE_RIGHTS             0x8     /* Attached rights are freed */
> >
> >  #define        sotounpcb(so)   ((struct unpcb *)((so)->so_pcb))
> >
> > _______________________________________________
> > svn-src-head at freebsd.org mailing list
> > https://lists.freebsd.org/mailman/listinfo/svn-src-head
> > To unsubscribe, send any mail to "svn-src-head-unsubscribe at freebsd.org"