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

[Oliver Pinter]

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

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"