Page MenuHomeFreeBSD
Differential D26586 Diff 77629 sys/netgraph/ng_l2tp.c

Changeset View

Standalone View

View Options

sys/netgraph/ng_l2tp.c

Show First 20 Lines • Show All 650 Lines • ▼ Show 20 Linesng_l2tp_shutdown(node_p node)
struct l2tp_seq *const seq = &priv->seq; struct l2tp_seq *const seq = &priv->seq;
/* Sanity check */ /* Sanity check */
L2TP_SEQ_CHECK(seq); L2TP_SEQ_CHECK(seq);
/* Reset sequence number state */ /* Reset sequence number state */
ng_l2tp_seq_reset(priv); ng_l2tp_seq_reset(priv);
/* Free private data if neither timer is running */ /* Wait for callouts to finish executing before freeing anything. */
ng_uncallout(&seq->rack_timer, node); callout_drain(&seq->rack_timer);
ng_uncallout(&seq->xack_timer, node); callout_drain(&seq->xack_timer);
donnerUnsubmitted
Not Done
Inline Actions

ng_uncallout calls NG_FREE_ITEM(...->c_arg), too.
How is this handled now? By waiting for the callout to do it itself?

donner: ng_uncallout calls NG_FREE_ITEM(...->c_arg), too. How is this handled now? By waiting for the…
markjUnsubmitted
Done
Inline Actions

Hmm, good question. This case is handled in the netgraph framework: when you use ng_callout(), a callout is scheduled using ng_callout_trampoline(), which uses a NGQF_FN message to execute the timeout handler in the context of a netgraph node. ng_apply_item() handles freeing the item after calling the function. So ng_uncallout() frees the item if it stopped the callout before it started executing, otherwise it is freed by the callout handler itself.

Note that ng_l2tp_seq_reset() calls ng_uncallout() already to stop the handlers. If that catches the handlers before they check callout_active() then they're ok. However, ng_l2tp_seq_rack_timeout() reschedules itself. So there is a (very narrow) race:

CPU 1:

  • in ng_l2tp_seq_rack_timeout()
  • reschedules the callout, allocating an item
  • drops seq->mtx, but does not return yet

CPU 2:

  • calls ng_l2tp_seq_reset(), calls ng_uncallout(&seq->rack_timer)
  • callout_stop() returns 0 because rack_timer is still executing
  • ng_uncallout() does not free the item allocated by CPU 1 above
  • item is leaked

I will think about it some more, but I think this problem is general to ng_callout()/ng_uncallout().

markj: Hmm, good question. This case is handled in the netgraph framework: when you use ng_callout()…
mtx_destroy(&seq->mtx); mtx_destroy(&seq->mtx);
free(priv, M_NETGRAPH_L2TP); free(priv, M_NETGRAPH_L2TP);
/* Unref node */ /* Unref node */
NG_NODE_UNREF(node); NG_NODE_UNREF(node);
return (0); return (0);
▲ Show 20 Lines • Show All 599 Lines • ▼ Show 20 Lines
{ {
struct l2tp_seq *const seq = &priv->seq; struct l2tp_seq *const seq = &priv->seq;
hook_p hook; hook_p hook;
int i; int i;
/* Sanity check */ /* Sanity check */
L2TP_SEQ_CHECK(seq); L2TP_SEQ_CHECK(seq);
mtx_lock(&seq->mtx);
/* Stop timers */ /* Stop timers */
ng_uncallout(&seq->rack_timer, priv->node); ng_uncallout(&seq->rack_timer, priv->node);
ng_uncallout(&seq->xack_timer, priv->node); ng_uncallout(&seq->xack_timer, priv->node);
/* Free retransmit queue */ /* Free retransmit queue */
for (i = 0; i < L2TP_MAX_XWIN; i++) { for (i = 0; i < L2TP_MAX_XWIN; i++) {
if (seq->xwin[i] == NULL) if (seq->xwin[i] == NULL)
break; break;
Show All 9 Linesng_l2tp_seq_reset(priv_p priv)
seq->rack = 0; seq->rack = 0;
seq->xack = 0; seq->xack = 0;
seq->wmax = L2TP_MAX_XWIN; seq->wmax = L2TP_MAX_XWIN;
seq->cwnd = 1; seq->cwnd = 1;
seq->ssth = seq->wmax; seq->ssth = seq->wmax;
seq->acks = 0; seq->acks = 0;
seq->rexmits = 0; seq->rexmits = 0;
bzero(seq->xwin, sizeof(seq->xwin)); bzero(seq->xwin, sizeof(seq->xwin));
mtx_unlock(&seq->mtx);
/* Done */ /* Done */
L2TP_SEQ_CHECK(seq); L2TP_SEQ_CHECK(seq);
} }
/* /*
* Handle receipt of an acknowledgement value (Nr) from peer. * Handle receipt of an acknowledgement value (Nr) from peer.
*/ */
▲ Show 20 Lines • Show All 315 Lines • Show Last 20 Lines