Date: Mon, 27 Jun 2011 11:10:15 +0000 (UTC) From: "Bjoern A. Zeeb" <bz@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r223588 - stable/8/sys/net Message-ID: <201106271110.p5RBAFq2095115@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bz Date: Mon Jun 27 11:10:15 2011 New Revision: 223588 URL: http://svn.freebsd.org/changeset/base/223588 Log: MFC r223223: 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 Modified: stable/8/sys/net/if_gre.c stable/8/sys/net/if_gre.h Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) Modified: stable/8/sys/net/if_gre.c ============================================================================== --- stable/8/sys/net/if_gre.c Mon Jun 27 10:42:06 2011 (r223587) +++ stable/8/sys/net/if_gre.c Mon Jun 27 11:10:15 2011 (r223588) @@ -55,6 +55,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> @@ -98,6 +99,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. @@ -203,7 +212,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; @@ -247,23 +255,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) && @@ -451,7 +513,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: stable/8/sys/net/if_gre.h ============================================================================== --- stable/8/sys/net/if_gre.h Mon Jun 27 10:42:06 2011 (r223587) +++ stable/8/sys/net/if_gre.h Mon Jun 27 11:10:15 2011 (r223588) @@ -68,8 +68,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?201106271110.p5RBAFq2095115>