Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 18 Jun 2011 09:34:03 +0000 (UTC)
From:      "Bjoern A. Zeeb" <bz@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r223223 - head/sys/net
Message-ID:  <201106180934.p5I9Y3KG030924@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: bz
Date: Sat Jun 18 09:34:03 2011
New Revision: 223223
URL: http://svn.freebsd.org/changeset/base/223223

Log:
  gre(4) was using a field in the softc to detect possible recursion.
  On MP systems this is not a usable solution anymore and could easily
  lead to false positives triggering enough logging that even  using
  the console was no longer usable (multiple parallel ping -f can do).
  
  Switch to the suggested solution of using mbuf tags to carry per
  packet state between gre_output() invocations.  Contrary to the
  proposed solution modelled after gif(4) only allocate one mbuf tag
  per packet rather than per packet and per gre_output() pass through.
  
  As the sysctl to control the possible valid (gre in gre) nestings does
  no sanity checks, make sure to always allocate space in the mbuf tag
  for at least one, and at most 255 possible gre interfaces to detect
  loops in addition to the counter.
  
  Submitted by:	Cristian KLEIN (cristi net.utcluj.ro) (original version)
  PR:		kern/114714
  Reviewed by:	Cristian KLEIN (cristi net.utcluj.ro)
  Reviewed bu:	Wooseog Choi (ben_choi hotmail.com)
  Sponsored by:	Sandvine Incorporated
  MFC after:	1 week

Modified:
  head/sys/net/if_gre.c
  head/sys/net/if_gre.h

Modified: head/sys/net/if_gre.c
==============================================================================
--- head/sys/net/if_gre.c	Sat Jun 18 07:47:15 2011	(r223222)
+++ head/sys/net/if_gre.c	Sat Jun 18 09:34:03 2011	(r223223)
@@ -48,6 +48,7 @@
 #include <sys/param.h>
 #include <sys/jail.h>
 #include <sys/kernel.h>
+#include <sys/libkern.h>
 #include <sys/malloc.h>
 #include <sys/module.h>
 #include <sys/mbuf.h>
@@ -91,6 +92,14 @@
 
 #define GRENAME	"gre"
 
+#define	MTAG_COOKIE_GRE		1307983903
+#define	MTAG_GRE_NESTING	1
+struct mtag_gre_nesting {
+	uint16_t	count;
+	uint16_t	max;
+	struct ifnet	*ifp[];
+};
+
 /*
  * gre_mtx protects all global variables in if_gre.c.
  * XXX: gre_softc data not protected yet.
@@ -196,7 +205,6 @@ gre_clone_create(ifc, unit, params)
 	sc->g_proto = IPPROTO_GRE;
 	GRE2IFP(sc)->if_flags |= IFF_LINK0;
 	sc->encap = NULL;
-	sc->called = 0;
 	sc->gre_fibnum = curthread->td_proc->p_fibnum;
 	sc->wccp_ver = WCCP_V1;
 	sc->key = 0;
@@ -240,23 +248,77 @@ gre_output(struct ifnet *ifp, struct mbu
 	struct gre_softc *sc = ifp->if_softc;
 	struct greip *gh;
 	struct ip *ip;
+	struct m_tag *mtag;
+	struct mtag_gre_nesting *gt;
+	size_t len;
 	u_short gre_ip_id = 0;
 	uint8_t gre_ip_tos = 0;
 	u_int16_t etype = 0;
 	struct mobile_h mob_h;
 	u_int32_t af;
-	int extra = 0;
+	int extra = 0, max;
 
 	/*
-	 * gre may cause infinite recursion calls when misconfigured.
-	 * We'll prevent this by introducing upper limit.
+	 * gre may cause infinite recursion calls when misconfigured.  High
+	 * nesting level may cause stack exhaustion.  We'll prevent this by
+	 * detecting loops and by introducing upper limit.
 	 */
-	if (++(sc->called) > max_gre_nesting) {
-		printf("%s: gre_output: recursively called too many "
-		       "times(%d)\n", if_name(GRE2IFP(sc)), sc->called);
-		m_freem(m);
-		error = EIO;    /* is there better errno? */
-		goto end;
+	mtag = m_tag_locate(m, MTAG_COOKIE_GRE, MTAG_GRE_NESTING, NULL);
+	if (mtag != NULL) {
+		struct ifnet **ifp2;
+
+		gt = (struct mtag_gre_nesting *)(mtag + 1);
+		gt->count++;
+		if (gt->count > min(gt->max,max_gre_nesting)) {
+			printf("%s: hit maximum recursion limit %u on %s\n",
+				__func__, gt->count - 1, ifp->if_xname);
+			m_freem(m);
+			error = EIO;	/* is there better errno? */
+			goto end;
+		}
+
+		ifp2 = gt->ifp;
+		for (max = gt->count - 1; max > 0; max--) {
+			if (*ifp2 == ifp)
+				break;
+			ifp2++;
+		}
+		if (*ifp2 == ifp) {
+			printf("%s: detected loop with nexting %u on %s\n",
+				__func__, gt->count-1, ifp->if_xname);
+			m_freem(m);
+			error = EIO;	/* is there better errno? */
+			goto end;
+		}
+		*ifp2 = ifp;
+
+	} else {
+		/*
+		 * Given that people should NOT increase max_gre_nesting beyond
+		 * their real needs, we allocate once per packet rather than
+		 * allocating an mtag once per passing through gre.
+		 *
+		 * Note: the sysctl does not actually check for saneness, so we
+		 * limit the maximum numbers of possible recursions here.
+		 */
+		max = imin(max_gre_nesting, 256);
+		/* If someone sets the sysctl <= 0, we want at least 1. */
+		max = imax(max, 1);
+		len = sizeof(struct mtag_gre_nesting) +
+		    max * sizeof(struct ifnet *);
+		mtag = m_tag_alloc(MTAG_COOKIE_GRE, MTAG_GRE_NESTING, len,
+		    M_NOWAIT);
+		if (mtag == NULL) {
+			m_freem(m);
+			error = ENOMEM;
+			goto end;
+		}
+		gt = (struct mtag_gre_nesting *)(mtag + 1);
+		bzero(gt, len);
+		gt->count = 1;
+		gt->max = max;
+		*gt->ifp = ifp;
+		m_tag_prepend(m, mtag);
 	}
 
 	if (!((ifp->if_flags & IFF_UP) &&
@@ -444,7 +506,6 @@ gre_output(struct ifnet *ifp, struct mbu
 	error = ip_output(m, NULL, &sc->route, IP_FORWARDING,
 	    (struct ip_moptions *)NULL, (struct inpcb *)NULL);
   end:
-	sc->called = 0;
 	if (error)
 		ifp->if_oerrors++;
 	return (error);

Modified: head/sys/net/if_gre.h
==============================================================================
--- head/sys/net/if_gre.h	Sat Jun 18 07:47:15 2011	(r223222)
+++ head/sys/net/if_gre.h	Sat Jun 18 09:34:03 2011	(r223223)
@@ -61,8 +61,6 @@ struct gre_softc {
 
 	const struct encaptab *encap;	/* encapsulation cookie */
 
-	int called;		/* infinite recursion preventer */
-
 	uint32_t key;		/* key included in outgoing GRE packets */
 				/* zero means none */
 



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