⬢ DragonFlyBSD Kernel Audit
DF-3495 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/netgraph/l2tp/ng_l2tp.c b/sys/netgraph/l2tp/ng_l2tp.c
--- a/sys/netgraph/l2tp/ng_l2tp.c
+++ b/sys/netgraph/l2tp/ng_l2tp.c
@@ -150,7 +150,9 @@ struct ng_l2tp_private {
 	struct ng_l2tp_config	conf;		/* node configuration */
 	struct ng_l2tp_stats	stats;		/* node statistics */
 	struct l2tp_seq		seq;		/* ctrl sequence number state */
-	char			*ftarget;	/* failure message target */
+	char			*ftarget;	/* failure message target */
+	LIST_HEAD(, ng_l2tp_hook_private) dead_hpriv; /* hooks detached but
+					possibly still referenced in-flight */
 };
 typedef struct ng_l2tp_private *priv_p;
@@ -158,7 +160,8 @@ typedef struct ng_l2tp_private *priv_p;
 struct ng_l2tp_hook_private {
 	struct ng_l2tp_sess_config	conf;	/* hook/session config */
 	u_int16_t			ns;	/* data ns sequence number */
 	u_int16_t			nr;	/* data nr sequence number */
+	LIST_ENTRY(ng_l2tp_hook_private) dead_next; /* deferred-free chain */
 };
 typedef struct ng_l2tp_hook_private *hookpriv_p;
@@ -765,6 +768,11 @@ ng_l2tp_recv_lower(node_p node, struct mbuf *m, meta_p meta)
 		if (hook == NULL) {
 			priv->stats.recvUnknownSID++;
 			NG_FREE_DATA(m, meta);
 			return (ENOTCONN);
 		}
+		/*
+		 * DF-3495: the hook list and the hook private data are
+		 * mutated without synchronization by ng_l2tp_disconnect()
+		 * (rmhook).  Hold a reference on the hook for the duration
+		 * of this packet's processing so the hook struct cannot be
+		 * freed underneath us, and re-check validity before use.
+		 */
+		NG_HOOK_REF(hook);
 		hpriv = NG_HOOK_PRIVATE(hook);
 	}
@@ -879,6 +887,17 @@ ng_l2tp_recv_lower(node_p node, struct mbuf *m, meta_p meta)
 	/* Deliver data */
 	NG_SEND_DATA(error, hook, m, meta);
+	NG_HOOK_UNREF(hook);
 	return (error);
 }
--- NOTE (also required): ng_l2tp_disconnect() must stop freeing hpriv
--- immediately and instead move it onto priv->dead_hpriv (freed at node
--- shutdown), because ->disconnect() runs before the final hook unref.
--- Combined sketch:
--- 	else {
--- 		hookpriv_p h = NG_HOOK_PRIVATE(hook);
--- 		NG_HOOK_SET_PRIVATE(hook, NULL);
--- 		if (h != NULL)
--- 			LIST_INSERT_HEAD(&priv->dead_hpriv, h, dead_next);
--- 	}
--- and ng_l2tp_shutdown() frees the dead_hpriv list.  With the hook ref
--- held above, the hook struct itself survives; NG_SEND_DATA() drops the
--- packet safely (ENOTCONN) once HK_INVALID is set by ng_destroy_hook().