--- a/sys/net/pfil.c +++ b/sys/net/pfil.c @@ -44,6 +44,8 @@ #include #include +#include +#include + #include #include #include @@ -66,6 +68,17 @@ struct netmsg_pfil { struct pfil_head *pfil_ph; }; +/* + * DF-3453: serializes hook-list walks (pfil_run_hooks) against the COW + * list replacement and the post-sync free performed by + * pfil_{add,remove}_hook_dispatch. netmsg_service_sync() only + * synchronizes with the netisr threads (netisr.c registers exactly + * those in netreglist), but pfil walks also execute on other threads: + * - softclock: ip_mroute tbf_reprocess_q callout -> tbf_process_q -> + * tbf_send_packet -> ip_output -> pfil_run_hooks(PFIL_OUT) + * - driver interrupt threads with IFF_IDIRECT (ether_input_handler + * direct dispatch -> bridge_pfil -> pfil_run_hooks(PFIL_IN)) + * A walk in flight on one of those threads keeps iterating the old + * list after pfil_list_free() recycles it -> use-after-free with an + * indirect call through pfh->pfil_func. + */ +static struct lwkt_token pfil_token = LWKT_TOKEN_INITIALIZER(pfil_token); + static LIST_HEAD(, pfil_head) pfil_head_list = LIST_HEAD_INITIALIZER(&pfil_head_list); @@ -106,11 +119,13 @@ pfil_run_hooks(struct pfil_head *ph, struct mbuf **mp, struct ifnet *ifp, /* Make sure 'list' is really used. */ cpu_ccfence(); + lwkt_gettoken(&pfil_token); TAILQ_FOREACH(pfh, list, pfil_link) { if (pfh->pfil_func != NULL) { rv = pfh->pfil_func(pfh->pfil_arg, &m, ifp, dir); if (rv != 0 || m == NULL) break; } } + lwkt_reltoken(&pfil_token); *mp = m; return (rv); @@ -208,6 +223,8 @@ pfil_add_hook_dispatch(netmsg_t nmsg) /* * Duplicate the requested lists, install new hooks * on the duplication */ + lwkt_gettoken(&pfil_token); + if (flags & PFIL_IN) { list_in = pfil_list_alloc(); pfil_list_dup(ph->ph_in, list_in, NULL); @@ -242,6 +259,8 @@ pfil_add_hook_dispatch(netmsg_t nmsg) if (old_list_in != NULL) pfil_list_free(old_list_in); if (old_list_out != NULL) pfil_list_free(old_list_out); + + lwkt_reltoken(&pfil_token); reply: lwkt_replymsg(&nmsg->base.lmsg, err); } @@ -314,6 +333,8 @@ pfil_remove_hook_dispatch(netmsg_t nmsg) /* * Duplicate the requested lists, but the pfil hook to * be deleted is not copied */ + lwkt_gettoken(&pfil_token); + if (flags & PFIL_IN) { KKASSERT(skip_in != NULL); list_in = pfil_list_alloc(); @@ -350,6 +371,8 @@ pfil_remove_hook_dispatch(netmsg_t nmsg) if (old_list_in != NULL) pfil_list_free(old_list_in); if (old_list_out != NULL) pfil_list_free(old_list_out); + + lwkt_reltoken(&pfil_token); reply: lwkt_replymsg(&nmsg->base.lmsg, err); }