Fix cleanup race between unp_dispose and unp_gc
unp_dispose and unp_gc could race to teardown the same mbuf chains, which can lead to dereferencing freed filedesc pointers. This patch adds an IGNORE_RIGHTS flag on unpcbs marking the unpcb's RIGHTS as invalid/freed. The flag is protected by UNP_LIST_LOCK. To serialize against unp_gc, unp_dispose needs the socket object. Change the dom_dispose() KPI to take a socket object instead of an mbuf chain directly. PR: 194264 Differential Revision: https://reviews.freebsd.org/D3044 Reviewed by: mjg (earlier version) Approved by: markj (mentor) Obtained from: mjg MFC after: 1 month Sponsored by: EMC / Isilon Storage Division
This commit is contained in:
parent
36632bef0c
commit
0c40f3532d
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=285522
@ -805,7 +805,7 @@ sofree(struct socket *so)
|
||||
|
||||
VNET_SO_ASSERT(so);
|
||||
if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose != NULL)
|
||||
(*pr->pr_domain->dom_dispose)(so->so_rcv.sb_mb);
|
||||
(*pr->pr_domain->dom_dispose)(so);
|
||||
if (pr->pr_usrreqs->pru_detach != NULL)
|
||||
(*pr->pr_usrreqs->pru_detach)(so);
|
||||
|
||||
@ -2356,7 +2356,7 @@ sorflush(struct socket *so)
|
||||
{
|
||||
struct sockbuf *sb = &so->so_rcv;
|
||||
struct protosw *pr = so->so_proto;
|
||||
struct sockbuf asb;
|
||||
struct socket aso;
|
||||
|
||||
VNET_SO_ASSERT(so);
|
||||
|
||||
@ -2381,8 +2381,9 @@ sorflush(struct socket *so)
|
||||
* and mutex data unchanged.
|
||||
*/
|
||||
SOCKBUF_LOCK(sb);
|
||||
bzero(&asb, offsetof(struct sockbuf, sb_startzero));
|
||||
bcopy(&sb->sb_startzero, &asb.sb_startzero,
|
||||
bzero(&aso, sizeof(aso));
|
||||
aso.so_pcb = so->so_pcb;
|
||||
bcopy(&sb->sb_startzero, &aso.so_rcv.sb_startzero,
|
||||
sizeof(*sb) - offsetof(struct sockbuf, sb_startzero));
|
||||
bzero(&sb->sb_startzero,
|
||||
sizeof(*sb) - offsetof(struct sockbuf, sb_startzero));
|
||||
@ -2390,12 +2391,12 @@ sorflush(struct socket *so)
|
||||
sbunlock(sb);
|
||||
|
||||
/*
|
||||
* Dispose of special rights and flush the socket buffer. Don't call
|
||||
* any unsafe routines (that rely on locks being initialized) on asb.
|
||||
* Dispose of special rights and flush the copied socket. Don't call
|
||||
* any unsafe routines (that rely on locks being initialized) on aso.
|
||||
*/
|
||||
if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose != NULL)
|
||||
(*pr->pr_domain->dom_dispose)(asb.sb_mb);
|
||||
sbrelease_internal(&asb, so);
|
||||
(*pr->pr_domain->dom_dispose)(&aso);
|
||||
sbrelease_internal(&aso.so_rcv, so);
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -278,6 +278,7 @@ static int unp_connectat(int, struct socket *, struct sockaddr *,
|
||||
static int unp_connect2(struct socket *so, struct socket *so2, int);
|
||||
static void unp_disconnect(struct unpcb *unp, struct unpcb *unp2);
|
||||
static void unp_dispose(struct mbuf *);
|
||||
static void unp_dispose_so(struct socket *so);
|
||||
static void unp_shutdown(struct unpcb *);
|
||||
static void unp_drop(struct unpcb *, int);
|
||||
static void unp_gc(__unused void *, int);
|
||||
@ -334,7 +335,7 @@ static struct domain localdomain = {
|
||||
.dom_name = "local",
|
||||
.dom_init = unp_init,
|
||||
.dom_externalize = unp_externalize,
|
||||
.dom_dispose = unp_dispose,
|
||||
.dom_dispose = unp_dispose_so,
|
||||
.dom_protosw = localsw,
|
||||
.dom_protoswNPROTOSW = &localsw[sizeof(localsw)/sizeof(localsw[0])]
|
||||
};
|
||||
@ -2216,15 +2217,19 @@ unp_gc_process(struct unpcb *unp)
|
||||
* Mark all sockets we reference with RIGHTS.
|
||||
*/
|
||||
so = unp->unp_socket;
|
||||
SOCKBUF_LOCK(&so->so_rcv);
|
||||
unp_scan(so->so_rcv.sb_mb, unp_accessable);
|
||||
SOCKBUF_UNLOCK(&so->so_rcv);
|
||||
if ((unp->unp_gcflag & UNPGC_IGNORE_RIGHTS) == 0) {
|
||||
SOCKBUF_LOCK(&so->so_rcv);
|
||||
unp_scan(so->so_rcv.sb_mb, unp_accessable);
|
||||
SOCKBUF_UNLOCK(&so->so_rcv);
|
||||
}
|
||||
|
||||
/*
|
||||
* Mark all sockets in our accept queue.
|
||||
*/
|
||||
ACCEPT_LOCK();
|
||||
TAILQ_FOREACH(soa, &so->so_comp, so_list) {
|
||||
if ((sotounpcb(soa)->unp_gcflag & UNPGC_IGNORE_RIGHTS) != 0)
|
||||
continue;
|
||||
SOCKBUF_LOCK(&soa->so_rcv);
|
||||
unp_scan(soa->so_rcv.sb_mb, unp_accessable);
|
||||
SOCKBUF_UNLOCK(&soa->so_rcv);
|
||||
@ -2254,11 +2259,13 @@ unp_gc(__unused void *arg, int pending)
|
||||
unp_taskcount++;
|
||||
UNP_LIST_LOCK();
|
||||
/*
|
||||
* First clear all gc flags from previous runs.
|
||||
* First clear all gc flags from previous runs, apart from
|
||||
* UNPGC_IGNORE_RIGHTS.
|
||||
*/
|
||||
for (head = heads; *head != NULL; head++)
|
||||
LIST_FOREACH(unp, *head, unp_link)
|
||||
unp->unp_gcflag = 0;
|
||||
unp->unp_gcflag =
|
||||
(unp->unp_gcflag & UNPGC_IGNORE_RIGHTS);
|
||||
|
||||
/*
|
||||
* Scan marking all reachable sockets with UNPGC_REF. Once a socket
|
||||
@ -2335,6 +2342,21 @@ unp_dispose(struct mbuf *m)
|
||||
unp_scan(m, unp_freerights);
|
||||
}
|
||||
|
||||
/*
|
||||
* Synchronize against unp_gc, which can trip over data as we are freeing it.
|
||||
*/
|
||||
static void
|
||||
unp_dispose_so(struct socket *so)
|
||||
{
|
||||
struct unpcb *unp;
|
||||
|
||||
unp = sotounpcb(so);
|
||||
UNP_LIST_LOCK();
|
||||
unp->unp_gcflag |= UNPGC_IGNORE_RIGHTS;
|
||||
UNP_LIST_UNLOCK();
|
||||
unp_dispose(so->so_rcv.sb_mb);
|
||||
}
|
||||
|
||||
static void
|
||||
unp_scan(struct mbuf *m0, void (*op)(struct filedescent **, int))
|
||||
{
|
||||
|
@ -42,6 +42,7 @@
|
||||
*/
|
||||
struct mbuf;
|
||||
struct ifnet;
|
||||
struct socket;
|
||||
|
||||
struct domain {
|
||||
int dom_family; /* AF_xxx */
|
||||
@ -53,7 +54,7 @@ struct domain {
|
||||
int (*dom_externalize) /* externalize access rights */
|
||||
(struct mbuf *, struct mbuf **, int);
|
||||
void (*dom_dispose) /* dispose of internalized rights */
|
||||
(struct mbuf *);
|
||||
(struct socket *);
|
||||
struct protosw *dom_protosw, *dom_protoswNPROTOSW;
|
||||
struct domain *dom_next;
|
||||
int (*dom_rtattach) /* initialize routing table */
|
||||
|
@ -106,6 +106,7 @@ struct unpcb {
|
||||
#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()
|
||||
|
Loading…
Reference in New Issue
Block a user