svn commit: r353635 - in head/sys: netinet netinet6
Gleb Smirnoff
glebius at freebsd.org
Wed Oct 16 16:57:24 UTC 2019
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 +0000, 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 <sys/lock.h>
H> #include <sys/mutex.h>
H> #include <sys/sysctl.h>
H> +#include <sys/socket.h>
H>
H> +#include <net/if.h>
H> +#include <net/if_var.h>
H> #include <net/rss_config.h>
H> #include <net/netisr.h>
H> #include <net/vnet.h>
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> ==============================================================================
H> --- head/sys/netinet6/frag6.c Wed Oct 16 09:04:53 2019 (r353634)
H> +++ head/sys/netinet6/frag6.c Wed Oct 16 09:11:49 2019 (r353635)
H> @@ -246,7 +246,7 @@ frag6_freef(struct ip6q *q6, uint32_t bucket)
H> * Return ICMP time exceeded error for the 1st fragment.
H> * Just free other fragments.
H> */
H> - if (af6->ip6af_off == 0) {
H> + if (af6->ip6af_off == 0 && m->m_pkthdr.rcvif != NULL) {
H>
H> /* Adjust pointer. */
H> ip6 = mtod(m, struct ip6_hdr *);
H> @@ -272,6 +272,43 @@ frag6_freef(struct ip6q *q6, uint32_t bucket)
H> }
H>
H> /*
H> + * Drain off all datagram fragments belonging to
H> + * the given network interface.
H> + */
H> +static void
H> +frag6_cleanup(void *arg __unused, struct ifnet *ifp)
H> +{
H> + struct ip6q *q6, *q6n, *head;
H> + struct ip6asfrag *af6;
H> + struct mbuf *m;
H> + int i;
H> +
H> + KASSERT(ifp != NULL, ("%s: ifp is NULL", __func__));
H> +
H> + CURVNET_SET_QUIET(ifp->if_vnet);
H> + for (i = 0; i < IP6REASS_NHASH; i++) {
H> + IP6QB_LOCK(i);
H> + head = IP6QB_HEAD(i);
H> + /* Scan fragment list. */
H> + for (q6 = head->ip6q_next; q6 != head; q6 = q6n) {
H> + q6n = q6->ip6q_next;
H> +
H> + for (af6 = q6->ip6q_down; af6 != (struct ip6asfrag *)q6;
H> + af6 = af6->ip6af_down) {
H> + m = IP6_REASS_MBUF(af6);
H> +
H> + /* clear no longer valid rcvif pointer */
H> + if (m->m_pkthdr.rcvif == ifp)
H> + m->m_pkthdr.rcvif = NULL;
H> + }
H> + }
H> + IP6QB_UNLOCK(i);
H> + }
H> + CURVNET_RESTORE();
H> +}
H> +EVENTHANDLER_DEFINE(ifnet_departure_event, frag6_cleanup, NULL, 0);
H> +
H> +/*
H> * Like in RFC2460, in RFC8200, fragment and reassembly rules do not agree with
H> * each other, in terms of next header field handling in fragment header.
H> * While the sender will use the same value for all of the fragmented packets,
H> @@ -307,6 +344,7 @@ int
H> frag6_input(struct mbuf **mp, int *offp, int proto)
H> {
H> struct ifnet *dstifp;
H> + struct ifnet *srcifp;
H> struct in6_ifaddr *ia6;
H> struct ip6_hdr *ip6;
H> struct ip6_frag *ip6f;
H> @@ -338,6 +376,11 @@ frag6_input(struct mbuf **mp, int *offp, int proto)
H> return (IPPROTO_DONE);
H> #endif
H>
H> + /*
H> + * Store receive network interface pointer for later.
H> + */
H> + srcifp = m->m_pkthdr.rcvif;
H> +
H> dstifp = NULL;
H> /* Find the destination interface of the packet. */
H> ia6 = in6ifa_ifwithaddr(&ip6->ip6_dst, 0 /* XXX */);
H> @@ -534,6 +577,9 @@ frag6_input(struct mbuf **mp, int *offp, int proto)
H> frag6_deq(af6, bucket);
H> free(af6, M_FRAG6);
H>
H> + /* Set a valid receive interface pointer. */
H> + merr->m_pkthdr.rcvif = srcifp;
H> +
H> /* Adjust pointer. */
H> ip6err = mtod(merr, struct ip6_hdr *);
H>
H> @@ -720,6 +766,8 @@ insert:
H> for (t = m; t; t = t->m_next)
H> plen += t->m_len;
H> m->m_pkthdr.len = plen;
H> + /* Set a valid receive interface pointer. */
H> + m->m_pkthdr.rcvif = srcifp;
H> }
H>
H> #ifdef RSS
H> _______________________________________________
H> svn-src-all at freebsd.org mailing list
H> https://lists.freebsd.org/mailman/listinfo/svn-src-all
H> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"
--
Gleb Smirnoff
More information about the svn-src-head
mailing list