From 38413aff485d800ac905ba71fa5f34460f976b26 Mon Sep 17 00:00:00 2001 From: bz Date: Mon, 27 Jun 2011 11:10:15 +0000 Subject: [PATCH] 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 git-svn-id: svn://svn.freebsd.org/base/stable/8@223588 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- sys/net/if_gre.c | 83 +++++++++++++++++++++++++++++++++++++++++------- sys/net/if_gre.h | 2 -- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/sys/net/if_gre.c b/sys/net/if_gre.c index 732380675..b01d6477f 100644 --- a/sys/net/if_gre.c +++ b/sys/net/if_gre.c @@ -55,6 +55,7 @@ #include #include #include +#include #include #include #include @@ -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 mbuf *m, struct sockaddr *dst, 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 mbuf *m, struct sockaddr *dst, 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); diff --git a/sys/net/if_gre.h b/sys/net/if_gre.h index 186d4cc6c..13b882c85 100644 --- a/sys/net/if_gre.h +++ b/sys/net/if_gre.h @@ -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 */ -- 2.45.0