--- a/sys/net/stf/if_stf.c +++ b/sys/net/stf/if_stf.c @@ -209,6 +209,20 @@ break; case MOD_UNLOAD: sc = stf; + /* + * DF-3433: tearing the softc down while the interface is + * configured (IFF_UP) races every in-flight consumer of + * the softc: the lock-free encaptab walk in encap4_input() + * (ip_encap.c), the asynchronous IPv6 input of already + * decapsulated packets (m_pkthdr.rcvif = &sc->sc_if + * survives netisr_queue), and the nd6 teardown itself. + * stf_encapcheck() only matches while IFF_UP is set, so + * refuse to unload a configured interface; the admin must + * take it down first (which stops new matches and lets the + * serializers drain). + */ + if (sc->sc_if.if_flags & IFF_UP) + return (EBUSY); bpfdetach(&sc->sc_if); if_detach(&sc->sc_if); err = encap_detach(sc->encap_cookie); --- a/sys/netinet/ip_encap.c +++ b/sys/netinet/ip_encap.c @@ -92,6 +92,13 @@ #include MALLOC_DEFINE(M_IPENCAP, "IP Encapsulation", "IP Encapsulation"); +/* + * The encaptab walk in encap4_input()/encap6_input() runs from netisr + * transport processing, i.e. with mp_token held. Mutators must take + * mp_token too so they cannot unlink+free an entry (or the caller's + * softc, via ep->arg) while a walk is in flight (DF-3425, DF-3433). + */ + static void encap_add (struct encaptab *); static int mask_match (const struct encaptab *, const struct sockaddr *, const struct sockaddr *); @@ -326,6 +333,7 @@ { struct encaptab *ep; + lwkt_gettoken(&mp_token); crit_enter(); /* sanity check on args */ if (sp->sa_len > sizeof ep->src || dp->sa_len > sizeof ep->dst) @@ -369,10 +377,12 @@ encap_add(ep); crit_exit(); + lwkt_reltoken(&mp_token); return ep; fail: crit_exit(); + lwkt_reltoken(&mp_token); return NULL; } @@ -383,6 +393,7 @@ { struct encaptab *ep; + lwkt_gettoken(&mp_token); crit_enter(); /* sanity check on args */ if (!func) @@ -401,10 +412,12 @@ encap_add(ep); crit_exit(); + lwkt_reltoken(&mp_token); return ep; fail: crit_exit(); + lwkt_reltoken(&mp_token); return NULL; } @@ -414,14 +427,17 @@ const struct encaptab *ep = cookie; struct encaptab *p; + lwkt_gettoken(&mp_token); for (p = LIST_FIRST(&encaptab); p; p = LIST_NEXT(p, chain)) { if (p == ep) { LIST_REMOVE(p, chain); kfree(p, M_IPENCAP); /*XXX*/ + lwkt_reltoken(&mp_token); return 0; } } + lwkt_reltoken(&mp_token); return EINVAL; } --- a/sys/net/ipfw3_basic/ip_fw3_state.c +++ b/sys/net/ipfw3_basic/ip_fw3_state.c @@ -315,9 +315,26 @@ } } if (*the_count <= the_max) { + s = RB_FIND(fw3_state_tree, the_tree, k); + if (s != NULL) { + /* + * Already tracked: limit-type rules (arg3 != 0) + * skip the RB_FIND above, so re-check here instead + * of allocating a duplicate the tree will reject + * (DF-3434: the rejected allocation was leaked and + * kprintf("oops") fired once per packet). + */ + s->timestamp = time_uptime; + goto done; + } (*the_count)++; s = kmalloc(LEN_FW3_STATE, M_IPFW3_STATE, M_INTWAIT | M_NULLOK | M_ZERO); + if (s == NULL) { + /* DF-3435: M_NULLOK permits NULL at the type + * limit; do not write through it */ + goto done; + } s->src_addr = k->src_addr; s->dst_addr = k->dst_addr; s->src_port = k->src_port; @@ -325,7 +342,7 @@ s->stub = *f; s->timestamp = time_uptime; if (RB_INSERT(fw3_state_tree, the_tree, s)) { - kprintf("oops\n"); + kfree(s, M_IPFW3_STATE); } } done: @@ -542,6 +559,11 @@ struct ipfw3_state_context *state_ctx = fw3_state_ctx[mycpuid]; struct ipfw3_state *s, *tmp; + if (state_ctx == NULL) { + /* DF-3436: fini_dispatch may have run first on this cpu */ + netisr_forwardmsg_all(&nmsg->base, mycpuid + 1); + return; + } RB_FOREACH_SAFE(s, fw3_state_tree, &state_ctx->rb_icmp_in, tmp) { if (time_uptime - s->timestamp > sysctl_var_icmp_timeout) { RB_REMOVE(fw3_state_tree, &state_ctx->rb_icmp_in, s); @@ -688,7 +710,9 @@ 0, ip_fw3_state_fini_dispatch); netisr_domsg(&msg, 0); - callout_stop(&ip_fw3_state_cleanup_callout); + /* DF-3436: the handler re-arms itself; cancel synchronously and + * disconnect the callout (re-initialized by the next MOD_LOAD) */ + callout_terminate(&ip_fw3_state_cleanup_callout); } void