DF-0503 / 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 | diff --git a/sys/netgraph7/ng_fec.c b/sys/netgraph7/ng_fec.c --- a/sys/netgraph7/ng_fec.c +++ b/sys/netgraph7/ng_fec.c @@ -101,6 +101,8 @@ #include <sys/queue.h> #include <net/if.h> +#include <net/if_var.h> +#include <sys/eventhandler.h> #include <net/if_dl.h> #include <net/if_types.h> #include <net/if_media.h> @@ -346,6 +348,38 @@ INTERFACE STUFF ************************************************************************/ +static void ng_fec_ifnet_detach_event(void *, struct ifnet *); + +/* + * ifnet_detach_event handler: walks every fec bundle and NULLs any + * stored member-ifp that matches the detaching interface. Without + * this, the stored p->fec_if becomes a dangling pointer that + * ng_fec_tick / ng_fec_input / ng_fec_start dereference via UAF. + * See DF-0503. + */ +static void +ng_fec_ifnet_detach_event(void *arg, struct ifnet *detaching_ifp) +{ + struct ng_fec_private *priv; + + mtx_lock(&ng_fec_mtx); + LIST_FOREACH(priv, &ng_fec_units, fec_next) { + struct ng_fec_bundle *b = &priv->fec_bundle; + struct ng_fec_portlist *p; + + TAILQ_FOREACH(p, &b->ng_fec_ports, fec_list) { + if (p->fec_if == detaching_ifp) { + kprintf("fec%d: member iface %s detached; " + "clearing stored pointer\n", + priv->unit, detaching_ifp->if_xname); + p->fec_if = NULL; + /* keep fec_if_input for restore-on-re-add */ + } + } + } + mtx_unlock(&ng_fec_mtx); +} + static int ng_fec_addport(struct ng_fec_private *priv, char *iface) { @@ -366,13 +400,17 @@ return (EINVAL); } - /* Find the interface */ + /* Find the interface. Per if_var.h:894 ifunit() requires + * ifnet_lock held before AND while accessing the returned ifp. */ + ifnet_lock(); bifp = ifunit(iface); if (bifp == NULL) { + ifnet_unlock(); kprintf("fec%d: tried to add iface %s, which " "doesn't seem to exist\n", priv->unit, iface); return(ENOENT); } + ifnet_unlock(); /* See if we have room in the bundle */ if (b->fec_ifcnt == FEC_BUNDLESIZ) { @@ -477,13 +515,16 @@ return (EINVAL); } - /* Find the interface */ + /* Find the interface (ifnet_lock required per if_var.h:894). */ + ifnet_lock(); bifp = ifunit(iface); if (bifp == NULL) { + ifnet_unlock(); kprintf("fec%d: tried to remove iface %s, which " "doesn't seem to exist\n", priv->unit, iface); return(ENOENT); } + ifnet_unlock(); TAILQ_FOREACH(p, &b->ng_fec_ports, fec_list) { if (p->fec_if == bifp) @@ -685,6 +726,9 @@ TAILQ_FOREACH(p, &b->ng_fec_ports, fec_list) { bzero((char *)&ifmr, sizeof(ifmr)); ifp = p->fec_if; + /* DF-0503: member iface may have been detached. */ + if (ifp == NULL) + continue; error = (*ifp->if_ioctl)(ifp, SIOCGIFMEDIA, (caddr_t)&ifmr); if (error) { kprintf("fec%d: failed to check status " @@ -1064,6 +1108,8 @@ } TAILQ_FOREACH(p, &b->ng_fec_ports, fec_list) { + if (p->fec_if == NULL) + continue; if (port == p->fec_idx) break; } @@ -1356,11 +1402,21 @@ { int error = 0; + static eventhandler_tag ifnet_detach_tag = NULL; + switch (event) { case MOD_LOAD: mtx_init(&ng_fec_mtx, "ng_fec", NULL, MTX_DEF); + /* DF-0503: register for interface departure events so we can + * NULL stored member-ifp pointers before the ifnet is freed. */ + ifnet_detach_tag = EVENTHANDLER_REGISTER(ifnet_detach_event, + ng_fec_ifnet_detach_event, NULL, EVENTHANDLER_PRI_ANY); break; case MOD_UNLOAD: + if (ifnet_detach_tag != NULL) { + EVENTHANDLER_DEREGISTER(ifnet_detach_event, ifnet_detach_tag); + ifnet_detach_tag = NULL; + } mtx_destroy(&ng_fec_mtx); break; default: |