svn commit: r303855 - in head/sys: kern sys
Oliver Pinter
oliver.pinter at hardenedbsd.org
Mon Aug 8 23:51:38 UTC 2016
(Added bdrewery to CC, since I'm talking with him on IRC.)
On Tue, Aug 9, 2016 at 1:43 AM, Mark Johnston <markj at freebsd.org> wrote:
> 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.
The question is how much external / out of tree components would use
this ABI or how acceptable to break this ABI.
I just grepped through the src tree for internal uses, and I found only these:
op at opn hardenedBSD.git> git grep dom_dispose
share/doc/smm/18.net/6.t: int (*dom_dispose)(); /*
dispose of internalized rights */
share/man/man9/domain.9: void (*dom_dispose) /*
dispose of internalized rights */
sys/kern/uipc_debug.c: db_printf("dom_dispose: %p\n", d->dom_dispose);
sys/kern/uipc_socket.c: if (pr->pr_flags & PR_RIGHTS &&
pr->pr_domain->dom_dispose != NULL)
sys/kern/uipc_socket.c: (*pr->pr_domain->dom_dispose)(so);
sys/kern/uipc_socket.c: * dom_dispose() and sbrelease_internal() are
an inlining of what was
sys/kern/uipc_socket.c: * In order to avoid calling dom_dispose with
the socket buffer mutex
sys/kern/uipc_socket.c: if (pr->pr_flags & PR_RIGHTS &&
pr->pr_domain->dom_dispose != NULL)
sys/kern/uipc_socket.c: (*pr->pr_domain->dom_dispose)(&aso);
sys/kern/uipc_usrreq.c: .dom_dispose = unp_dispose_so,
sys/sys/domain.h: void (*dom_dispose) /* dispose of
internalized rights */
>
>>
>> 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"
More information about the svn-src-head
mailing list