Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 18 Jun 2015 18:28:38 +0000 (UTC)
From:      "Andrey V. Elsukov" <ae@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r284566 - head/sys/netinet
Message-ID:  <201506181828.t5IIScG0060634@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ae
Date: Thu Jun 18 18:28:38 2015
New Revision: 284566
URL: https://svnweb.freebsd.org/changeset/base/284566

Log:
  Fix possible use after free in encap[46]_input().
  There is small window, when encap_detach() can free matched entry
  directly after we release encapmtx. Instead of use pointer to the
  matched entry, save pointers to needed variables from this entry
  and use them after release mutex.
  
  Pass argument stored in the encaptab entry to encap_fillarg(), instead
  of pointer to matched entry. Also do not allocate new mbuf tag, when
  argument that we plan to save in this tag is NULL.
  
  Also make encaptab variable static.
  
  Obtained from:	Yandex LLC
  Sponsored by:	Yandex LLC

Modified:
  head/sys/netinet/ip_encap.c

Modified: head/sys/netinet/ip_encap.c
==============================================================================
--- head/sys/netinet/ip_encap.c	Thu Jun 18 17:42:24 2015	(r284565)
+++ head/sys/netinet/ip_encap.c	Thu Jun 18 18:28:38 2015	(r284566)
@@ -95,14 +95,14 @@ static MALLOC_DEFINE(M_NETADDR, "encap_e
 static void encap_add(struct encaptab *);
 static int mask_match(const struct encaptab *, const struct sockaddr *,
 		const struct sockaddr *);
-static void encap_fillarg(struct mbuf *, const struct encaptab *);
+static void encap_fillarg(struct mbuf *, void *);
 
 /*
  * All global variables in ip_encap.c are locked using encapmtx.
  */
 static struct mtx encapmtx;
 MTX_SYSINIT(encapmtx, &encapmtx, "encapmtx", MTX_DEF);
-LIST_HEAD(, encaptab) encaptab = LIST_HEAD_INITIALIZER(encaptab);
+static LIST_HEAD(, encaptab) encaptab = LIST_HEAD_INITIALIZER(encaptab);
 
 /*
  * We currently keey encap_init() for source code compatibility reasons --
@@ -122,12 +122,12 @@ encap4_input(struct mbuf **mp, int *offp
 	struct sockaddr_in s, d;
 	const struct protosw *psw;
 	struct encaptab *ep, *match;
+	void *arg;
 	int matchprio, off, prio;
 
 	m = *mp;
 	off = *offp;
 	ip = mtod(m, struct ip *);
-	*mp = NULL;
 
 	bzero(&s, sizeof(s));
 	s.sin_family = AF_INET;
@@ -138,6 +138,8 @@ encap4_input(struct mbuf **mp, int *offp
 	d.sin_len = sizeof(struct sockaddr_in);
 	d.sin_addr = ip->ip_dst;
 
+	arg = NULL;
+	psw = NULL;
 	match = NULL;
 	matchprio = 0;
 	mtx_lock(&encapmtx);
@@ -182,14 +184,16 @@ encap4_input(struct mbuf **mp, int *offp
 			match = ep;
 		}
 	}
+	if (match != NULL) {
+		psw = match->psw;
+		arg = match->arg;
+	}
 	mtx_unlock(&encapmtx);
 
-	if (match) {
+	if (match != NULL) {
 		/* found a match, "match" has the best one */
-		psw = match->psw;
-		if (psw && psw->pr_input) {
-			encap_fillarg(m, match);
-			*mp = m;
+		if (psw != NULL && psw->pr_input != NULL) {
+			encap_fillarg(m, arg);
 			(*psw->pr_input)(mp, offp, proto);
 		} else
 			m_freem(m);
@@ -197,7 +201,6 @@ encap4_input(struct mbuf **mp, int *offp
 	}
 
 	/* last resort: inject to raw socket */
-	*mp = m;
 	return (rip_input(mp, offp, proto));
 }
 #endif
@@ -211,6 +214,7 @@ encap6_input(struct mbuf **mp, int *offp
 	struct sockaddr_in6 s, d;
 	const struct protosw *psw;
 	struct encaptab *ep, *match;
+	void *arg;
 	int prio, matchprio;
 
 	ip6 = mtod(m, struct ip6_hdr *);
@@ -224,6 +228,8 @@ encap6_input(struct mbuf **mp, int *offp
 	d.sin6_len = sizeof(struct sockaddr_in6);
 	d.sin6_addr = ip6->ip6_dst;
 
+	arg = NULL;
+	psw = NULL;
 	match = NULL;
 	matchprio = 0;
 	mtx_lock(&encapmtx);
@@ -251,17 +257,20 @@ encap6_input(struct mbuf **mp, int *offp
 			match = ep;
 		}
 	}
+	if (match != NULL) {
+		psw = match->psw;
+		arg = match->arg;
+	}
 	mtx_unlock(&encapmtx);
 
-	if (match) {
+	if (match != NULL) {
 		/* found a match */
-		psw = match->psw;
-		if (psw && psw->pr_input) {
-			encap_fillarg(m, match);
+		if (psw != NULL && psw->pr_input != NULL) {
+			encap_fillarg(m, arg);
 			return (*psw->pr_input)(mp, offp, proto);
 		} else {
 			m_freem(m);
-			return IPPROTO_DONE;
+			return (IPPROTO_DONE);
 		}
 	}
 
@@ -440,14 +449,16 @@ mask_match(const struct encaptab *ep, co
 }
 
 static void
-encap_fillarg(struct mbuf *m, const struct encaptab *ep)
+encap_fillarg(struct mbuf *m, void *arg)
 {
 	struct m_tag *tag;
 
-	tag = m_tag_get(PACKET_TAG_ENCAP, sizeof (void*), M_NOWAIT);
-	if (tag) {
-		*(void**)(tag+1) = ep->arg;
-		m_tag_prepend(m, tag);
+	if (arg != NULL) {
+		tag = m_tag_get(PACKET_TAG_ENCAP, sizeof(void *), M_NOWAIT);
+		if (tag != NULL) {
+			*(void**)(tag+1) = arg;
+			m_tag_prepend(m, tag);
+		}
 	}
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201506181828.t5IIScG0060634>