DF-3495 / fix.diff
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(). |