DF-3433 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 | --- 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 <sys/thread2.h> 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 |