DragonFlyBSD Kernel Audit
DF-2851 / fix.diff
← back to finding ↓ download raw
# DF-2851 fix: don't mutate netmsg_so_notify.base.nm_so while abortable
#
# Root cause: soaccept_predicate() "abuses" msg->base.nm_so as an output
# parameter, reassigning it to the just-accepted socket (uipc_syscalls.c:261)
# while the notify message is still queued/abortable and before the replier
# (sowakeup on a foreign netisr cpu) has set MSGF_REPLY.
# netmsg_so_notify_abort() (sys/kern/uipc_msg.c:744-753) re-derives BOTH the
# interlock pool token AND the ssb_mlist to unlink from nmsg->base.nm_so.
# When nm_so changes between the abort's lwkt_getpooltoken() and its later
# uses of nm_so (list selection + release), the abort runs unserialized
# against the foreign-cpu completion: it releases a pool token it never
# acquired (observed panic: "lwkt_reltoken: illegal release" inside
# netmsg_so_notify_abort) and on non-INVARIANTS kernels additionally performs
# TAILQ_REMOVE() against the wrong socket's notify list plus a double
# lwkt_replymsg().
#
# Fix: carry the accepted socket in a dedicated predicate-output field
# (nm_result) and leave base.nm_so -- the lock/list identity used by the
# abort path -- immutable for the lifetime of the message.
#
# Apply inside the guest with:  cd /usr/src && git apply fix.diff

diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index 7b9322f..c26455c 100644
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -258,7 +258,7 @@ soaccept_predicate(struct netmsg_so_notify *msg)
 		lwkt_relpooltoken(head);
 
 		msg->base.lmsg.ms_error = 0;
-		msg->base.nm_so = so;
+		msg->nm_result = so;
 		return (TRUE);
 	}
 	lwkt_relpooltoken(head);
@@ -325,6 +325,7 @@ kern_accept(int s, int fflags, struct sockaddr **name, int *namelen, int *res,
 		boolean_t pred;
 
 		/* Initialize necessary parts for soaccept_predicate() */
+		msg.nm_result = NULL;
 		netmsg_init(&msg.base, head, &netisr_apanic_rport, 0, NULL);
 		msg.nm_fflags = fflags;
 
@@ -342,6 +343,7 @@ kern_accept(int s, int fflags, struct sockaddr **name, int *namelen, int *res,
 	}
 
 	/* optimize for uniprocessor case later XXX JH */
+	msg.nm_result = NULL;
 	netmsg_init_abortable(&msg.base, head, &curthread->td_msgport,
 			      0, netmsg_so_notify, netmsg_so_notify_doabort);
 	msg.nm_predicate = soaccept_predicate;
@@ -358,7 +360,7 @@ accepted:
 	 * NOTE! soaccept_predicate() ref'd so for us, and soaccept() expects
 	 * 	 to eat the ref and turn it into a descriptor.
 	 */
-	so = msg.base.nm_so;
+	so = msg.nm_result;
 
 	fflag = lfp->f_flag;
 
diff --git a/sys/net/netmsg.h b/sys/net/netmsg.h
index 36e5999..5d3c091 100644
--- a/sys/net/netmsg.h
+++ b/sys/net/netmsg.h
@@ -126,6 +126,7 @@ struct netmsg_so_notify {
 	msg_predicate_fn_t	nm_predicate;
 	int			nm_fflags; /* flags e.g. FNONBLOCK */
 	int			nm_etype;  /* receive or send event */
+	struct socket		*nm_result; /* predicate output: accepted so */
 	TAILQ_ENTRY(netmsg_so_notify) nm_list;
 };