Hans,
as far as I remember I was against this changeset and I had
several other developers agreed that this should be fixed in
different way. Why did you proceed with checking it in? :(
On Wed, Oct 16, 2019 at 09:11:50AM +, Hans Petter Selasky wrote:
H> Author: hselasky
H> Date: Wed Oct 16 09:11:49 2019
H> New Revision: 353635
H> URL: https://svnweb.freebsd.org/changeset/base/353635
H>
H> Log:
H> Fix panic in network stack due to use after free when receiving
H> partial fragmented packets before a network interface is detached.
H>
H> When sending IPv4 or IPv6 fragmented packets and a fragment is lost
H> before the network device is freed, the mbuf making up the fragment
H> will remain in the temporary hashed fragment list and cause a panic
H> when it times out due to accessing a freed network interface
H> structure.
H>
H>
H> 1) Make sure the m_pkthdr.rcvif always points to a valid network
H> interface. Else the rcvif field should be set to NULL.
H>
H> 2) Use the rcvif of the last received fragment as m_pkthdr.rcvif for
H> the fully defragged packet, instead of the first received fragment.
H>
H> Panic backtrace for IPv6:
H>
H> panic()
H> icmp6_reflect() # tries to access rcvif->if_afdata[AF_INET6]->xxx
H> icmp6_error()
H> frag6_freef()
H> frag6_slowtimo()
H> pfslowtimo()
H> softclock_call_cc()
H> softclock()
H> ithread_loop()
H>
H> Reviewed by: bz
H> Differential Revision: https://reviews.freebsd.org/D19622
H> MFC after: 1 week
H> Sponsored by: Mellanox Technologies
H>
H> Modified:
H> head/sys/netinet/ip_reass.c
H> head/sys/netinet6/frag6.c
H>
H> Modified: head/sys/netinet/ip_reass.c
H>
==
H> --- head/sys/netinet/ip_reass.c Wed Oct 16 09:04:53 2019
(r353634)
H> +++ head/sys/netinet/ip_reass.c Wed Oct 16 09:11:49 2019
(r353635)
H> @@ -47,7 +47,10 @@ __FBSDID("$FreeBSD$");
H> #include
H> #include
H> #include
H> +#include
H>
H> +#include
H> +#include
H> #include
H> #include
H> #include
H> @@ -181,6 +184,7 @@ ip_reass(struct mbuf *m)
H> struct ip *ip;
H> struct mbuf *p, *q, *nq, *t;
H> struct ipq *fp;
H> +struct ifnet *srcifp;
H> struct ipqhead *head;
H> int i, hlen, next, tmpmax;
H> u_int8_t ecn, ecn0;
H> @@ -241,6 +245,11 @@ ip_reass(struct mbuf *m)
H> }
H>
H> /*
H> + * Store receive network interface pointer for later.
H> + */
H> +srcifp = m->m_pkthdr.rcvif;
H> +
H> +/*
H> * Attempt reassembly; if it succeeds, proceed.
H> * ip_reass() will return a different mbuf.
H> */
H> @@ -490,8 +499,11 @@ ip_reass(struct mbuf *m)
H> m->m_len += (ip->ip_hl << 2);
H> m->m_data -= (ip->ip_hl << 2);
H> /* some debugging cruft by sklower, below, will go away soon */
H> -if (m->m_flags & M_PKTHDR) /* XXX this should be done elsewhere */
H> +if (m->m_flags & M_PKTHDR) {/* XXX this should be done elsewhere */
H> m_fixhdr(m);
H> +/* set valid receive interface pointer */
H> +m->m_pkthdr.rcvif = srcifp;
H> +}
H> IPSTAT_INC(ips_reassembled);
H> IPQ_UNLOCK(hash);
H>
H> @@ -607,6 +619,43 @@ ipreass_drain(void)
H> }
H> }
H>
H> +/*
H> + * Drain off all datagram fragments belonging to
H> + * the given network interface.
H> + */
H> +static void
H> +ipreass_cleanup(void *arg __unused, struct ifnet *ifp)
H> +{
H> +struct ipq *fp, *temp;
H> +struct mbuf *m;
H> +int i;
H> +
H> +KASSERT(ifp != NULL, ("%s: ifp is NULL", __func__));
H> +
H> +/*
H> + * Skip processing if IPv4 reassembly is not initialised or
H> + * torn down by ipreass_destroy().
H> + */
H> +if (V_ipq_zone == NULL)
H> +return;
H> +
H> +CURVNET_SET_QUIET(ifp->if_vnet);
H> +for (i = 0; i < IPREASS_NHASH; i++) {
H> +IPQ_LOCK(i);
H> +/* Scan fragment list. */
H> +TAILQ_FOREACH_SAFE(fp, &V_ipq[i].head, ipq_list, temp) {
H> +for (m = fp->ipq_frags; m != NULL; m = m->m_nextpkt) {
H> +/* clear no longer valid rcvif pointer */
H> +if (m->m_pkthdr.rcvif == ifp)
H> +m->m_pkthdr.rcvif = NULL;
H> +}
H> +}
H> +IPQ_UNLOCK(i);
H> +}
H> +CURVNET_RESTORE();
H> +}
H> +EVENTHANDLER_DEFINE(ifnet_departure_event, ipreass_cleanup, NULL, 0);
H> +
H> #ifdef VIMAGE
H> /*
H> * Destroy IP reassembly structures.
H> @@ -617,6 +666,7 @@ ipreass_destroy(void)
H>
H> ipreass_drain();
H> uma_zdestroy(V_ipq_zone);
H> +V_ipq_zone = NULL;
H> for (int i = 0; i < IPREASS_NHASH; i++)
H> mtx_destroy(&V_ipq[i].lock);
H> }
H>
H> Modified: head/sys/netinet6/frag6.c
H>
