Rework socket upcalls to close some races with setup/teardown of upcalls.

- Each socket upcall is now invoked with the appropriate socket buffer
  locked.  It is not permissible to call soisconnected() with this lock
  held; however, so socket upcalls now return an integer value.  The two
  possible values are SU_OK and SU_ISCONNECTED.  If an upcall returns
  SU_ISCONNECTED, then the soisconnected() will be invoked on the
  socket after the socket buffer lock is dropped.
- A new API is provided for setting and clearing socket upcalls.  The
  API consists of soupcall_set() and soupcall_clear().
- To simplify locking, each socket buffer now has a separate upcall.
- When a socket upcall returns SU_ISCONNECTED, the upcall is cleared from
  the receive socket buffer automatically.  Note that a SO_SND upcall
  should never return SU_ISCONNECTED.
- All this means that accept filters should now return SU_ISCONNECTED
  instead of calling soisconnected() directly.  They also no longer need
  to explicitly clear the upcall on the new socket.
- The HTTP accept filter still uses soupcall_set() to manage its internal
  state machine, but other accept filters no longer have any explicit
  knowlege of socket upcall internals aside from their return value.
- The various RPC client upcalls currently drop the socket buffer lock
  while invoking soreceive() as a temporary band-aid.  The plan for
  the future is to add a new flag to allow soreceive() to be called with
  the socket buffer locked.
- The AIO callback for socket I/O is now also invoked with the socket
  buffer locked.  Previously sowakeup() would drop the socket buffer
  lock only to call aio_swake() which immediately re-acquired the socket
  buffer lock for the duration of the function call.

Discussed with:	rwatson, rmacklem

2 files changed, 18 insertions, 27 deletions

diff --git a/sys/netgraph/bluetooth/socket/ng_btsocket_rfcomm.c b/sys/netgraph/bluetooth/socket/ng_btsocket_rfcomm.c
index 0f409b5..4904c19 100644
--- a/sys/netgraph/bluetooth/socket/ng_btsocket_rfcomm.c
+++ b/sys/netgraph/bluetooth/socket/ng_btsocket_rfcomm.c
@@ -93,7 +93,7 @@ MALLOC_DEFINE(M_NETGRAPH_BTSOCKET_RFCOMM, "netgraph_btsocks_rfcomm",
 #define	ALOT	0x7fff
 
 /* Local prototypes */
-static void ng_btsocket_rfcomm_upcall
+static int ng_btsocket_rfcomm_upcall
 	(struct socket *so, void *arg, int waitflag);
 static void ng_btsocket_rfcomm_sessions_task
 	(void *ctx, int pending);
@@ -1007,7 +1007,7 @@ ng_btsocket_rfcomm_sockaddr(struct socket *so, struct sockaddr **nam)
  * Upcall function for L2CAP sockets. Enqueue RFCOMM task.
  */
 
-static void
+static int
 ng_btsocket_rfcomm_upcall(struct socket *so, void *arg, int waitflag)
 {
 	int	error;
@@ -1018,6 +1018,7 @@ ng_btsocket_rfcomm_upcall(struct socket *so, void *arg, int waitflag)
 	if ((error = ng_btsocket_rfcomm_task_wakeup()) != 0)
 		NG_BTSOCKET_RFCOMM_ALERT(
 "%s: Could not enqueue RFCOMM task, error=%d\n", __func__, error);
+	return (SU_OK);
 } /* ng_btsocket_rfcomm_upcall */
 
 /*
@@ -1047,13 +1048,11 @@ ng_btsocket_rfcomm_sessions_task(void *ctx, int pending)
 				panic("%s: DLC list is not empty\n", __func__);
 
 			/* Close L2CAP socket */
-			s->l2so->so_upcallarg = NULL;
-			s->l2so->so_upcall = NULL;
 			SOCKBUF_LOCK(&s->l2so->so_rcv);
-			s->l2so->so_rcv.sb_flags &= ~SB_UPCALL;
+			soupcall_clear(s->l2so, SO_RCV);
 			SOCKBUF_UNLOCK(&s->l2so->so_rcv);
 			SOCKBUF_LOCK(&s->l2so->so_snd);
-			s->l2so->so_snd.sb_flags &= ~SB_UPCALL;
+			soupcall_clear(s->l2so, SO_SND);
 			SOCKBUF_UNLOCK(&s->l2so->so_snd);
 			soclose(s->l2so);
 
@@ -1286,13 +1285,11 @@ ng_btsocket_rfcomm_session_create(ng_btsocket_rfcomm_session_p *sp,
 	LIST_INIT(&s->dlcs);
 
 	/* Prepare L2CAP socket */
-	l2so->so_upcallarg = NULL;
-	l2so->so_upcall = ng_btsocket_rfcomm_upcall;
 	SOCKBUF_LOCK(&l2so->so_rcv);
-	l2so->so_rcv.sb_flags |= SB_UPCALL;
+	soupcall_set(l2so, SO_RCV, ng_btsocket_rfcomm_upcall, NULL);
 	SOCKBUF_UNLOCK(&l2so->so_rcv);
 	SOCKBUF_LOCK(&l2so->so_snd);
-	l2so->so_snd.sb_flags |= SB_UPCALL;
+	soupcall_set(l2so, SO_SND, ng_btsocket_rfcomm_upcall, NULL);
 	SOCKBUF_UNLOCK(&l2so->so_snd);
 	l2so->so_state |= SS_NBIO;
 	s->l2so = l2so;
@@ -1370,13 +1367,11 @@ bad:
 	mtx_unlock(&s->session_mtx);
 
 	/* Return L2CAP socket back to its original state */
-	l2so->so_upcallarg = NULL;
-	l2so->so_upcall = NULL;
 	SOCKBUF_LOCK(&l2so->so_rcv);
-	l2so->so_rcv.sb_flags &= ~SB_UPCALL;
+	soupcall_clear(s->l2so, SO_RCV);
 	SOCKBUF_UNLOCK(&l2so->so_rcv);
 	SOCKBUF_LOCK(&l2so->so_snd);
-	l2so->so_snd.sb_flags &= ~SB_UPCALL;
+	soupcall_clear(s->l2so, SO_SND);
 	SOCKBUF_UNLOCK(&l2so->so_snd);
 	l2so->so_state &= ~SS_NBIO;
 
diff --git a/sys/netgraph/ng_ksocket.c b/sys/netgraph/ng_ksocket.c
index 312b5ef..821a1be 100644
--- a/sys/netgraph/ng_ksocket.c
+++ b/sys/netgraph/ng_ksocket.c
@@ -158,7 +158,7 @@ static const struct ng_ksocket_alias ng_ksocket_protos[] = {
 /* Helper functions */
 static int	ng_ksocket_check_accept(priv_p);
 static void	ng_ksocket_finish_accept(priv_p);
-static void	ng_ksocket_incoming(struct socket *so, void *arg, int waitflag);
+static int	ng_ksocket_incoming(struct socket *so, void *arg, int waitflag);
 static int	ng_ksocket_parse(const struct ng_ksocket_alias *aliases,
 			const char *s, int family);
 static void	ng_ksocket_incoming2(node_p node, hook_p hook,
@@ -616,13 +616,11 @@ ng_ksocket_connect(hook_p hook)
 	struct socket *const so = priv->so;
 
 	/* Add our hook for incoming data and other events */
-	priv->so->so_upcallarg = (caddr_t)node;
-	priv->so->so_upcall = ng_ksocket_incoming;
 	SOCKBUF_LOCK(&priv->so->so_rcv);
-	priv->so->so_rcv.sb_flags |= SB_UPCALL;
+	soupcall_set(priv->so, SO_RCV, ng_ksocket_incoming, node);
 	SOCKBUF_UNLOCK(&priv->so->so_rcv);
 	SOCKBUF_LOCK(&priv->so->so_snd);
-	priv->so->so_snd.sb_flags |= SB_UPCALL;
+	soupcall_set(priv->so, SO_SND, ng_ksocket_incoming, node);
 	SOCKBUF_UNLOCK(&priv->so->so_snd);
 	SOCK_LOCK(priv->so);
 	priv->so->so_state |= SS_NBIO;
@@ -941,12 +939,11 @@ ng_ksocket_shutdown(node_p node)
 	/* Close our socket (if any) */
 	if (priv->so != NULL) {
 		SOCKBUF_LOCK(&priv->so->so_rcv);
-		priv->so->so_rcv.sb_flags &= ~SB_UPCALL;
+		soupcall_clear(priv->so, SO_RCV);
 		SOCKBUF_UNLOCK(&priv->so->so_rcv);
 		SOCKBUF_LOCK(&priv->so->so_snd);
-		priv->so->so_snd.sb_flags &= ~SB_UPCALL;
+		soupcall_clear(priv->so, SO_SND);
 		SOCKBUF_UNLOCK(&priv->so->so_snd);
-		priv->so->so_upcall = NULL;
 		soclose(priv->so);
 		priv->so = NULL;
 	}
@@ -1000,7 +997,7 @@ ng_ksocket_disconnect(hook_p hook)
  * To decouple stack, we use queue version of ng_send_fn().
  */
 
-static void
+static int
 ng_ksocket_incoming(struct socket *so, void *arg, int waitflag)
 {
 	const node_p node = arg;
@@ -1017,6 +1014,7 @@ ng_ksocket_incoming(struct socket *so, void *arg, int waitflag)
 	    ng_send_fn1(node, NULL, &ng_ksocket_incoming2, so, 0, wait)) {
 		atomic_store_rel_int(&priv->fn_sent, 0);
 	}
+	return (SU_OK);
 }
 
 
@@ -1258,13 +1256,11 @@ ng_ksocket_finish_accept(priv_p priv)
 	 */
 	LIST_INSERT_HEAD(&priv->embryos, priv2, siblings);
 
-	so->so_upcallarg = (caddr_t)node;
-	so->so_upcall = ng_ksocket_incoming;
 	SOCKBUF_LOCK(&so->so_rcv);
-	so->so_rcv.sb_flags |= SB_UPCALL;
+	soupcall_set(so, SO_RCV, ng_ksocket_incoming, node);
 	SOCKBUF_UNLOCK(&so->so_rcv);
 	SOCKBUF_LOCK(&so->so_snd);
-	so->so_snd.sb_flags |= SB_UPCALL;
+	soupcall_set(so, SO_RCV, ng_ksocket_incoming, node);
 	SOCKBUF_UNLOCK(&so->so_snd);
 
 	/* Fill in the response data and send it or return it to the caller */