Date: Wed, 18 Jul 2007 23:51:04 GMT From: Cristian KLEIN <cristi@net.utcluj.ro> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/114714: [gre][patch] gre(4) is not MPSAFE and does not support keys Message-ID: <200707182351.l6INp4aa066745@www.freebsd.org> Resent-Message-ID: <200707190000.l6J00Afi008470@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 114714
>Category: kern
>Synopsis: [gre][patch] gre(4) is not MPSAFE and does not support keys
>Confidential: no
>Severity: serious
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Jul 19 00:00:09 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator: Cristian KLEIN
>Release: 7.0-CURRENT
>Organization:
Technical University of Cluj-Napoca
>Environment:
FreeBSD hades.local 7.0-CURRENT FreeBSD 7.0-CURRENT #11: Tue Jul 17 04:08:17 EEST 2007 cristi@hades.local:/usr/obj/usr/src/sys/GENERIC i386
>Description:
The first problem is that gre(4) uses a softc variable to check for loop prevention (sc->called). This is not MPSAFE, since multiple processors might use that variable at the same time. The patch below is adapted from gif(4) and uses mbuf_tags(9), which is MPSAFE.
Second, gre(4) does not support keys. The below patch contains all necessary modifications to man pages and code, to properly place keys on outbound. It has been successfully tested with a Cisco box.
>How-To-Repeat:
The first problem could be trigger by a high packet-per-second gre-encapsulated packets on a SMP machine.
For the second problem do the following on a Cisco box:
interface Tun 0
tunnel key 1234
Or for a Linux box:
ip tunnel ... key 1234 ...
>Fix:
The below patch solves both problems.
Patch attached with submission follows:
diff -ur src/sbin/ifconfig/ifconfig.8 src.new/sbin/ifconfig/ifconfig.8
--- sbin/ifconfig/ifconfig.8 2007-07-09 18:39:58.000000000 +0300
+++ sbin/ifconfig/ifconfig.8 2007-07-19 02:12:24.255339155 +0300
@@ -1646,6 +1646,16 @@
parameter.
.El
.Pp
+The following parameters are specific to GRE tunnel interfaces,
+.Xr gre 4 :
+.Bl -tag -width indent
+.It Cm grekey Ar key
+Configure the GRE key to be used for outgoing packets. Note that
+.Xr gre 4 will always accept GRE packets with invalid or absent keys.
+Also note that this command will change the MTU of the interface
+(currently 1476 without key, 1472 with key).
+.El
+.Pp
The following parameters are specific to
.Xr pfsync 4
interfaces:
diff -ur src/sbin/ifconfig/ifconfig.c src.new/sbin/ifconfig/ifconfig.c
--- sbin/ifconfig/ifconfig.c 2007-06-13 21:07:59.000000000 +0300
+++ sbin/ifconfig/ifconfig.c 2007-07-18 22:01:21.000000000 +0300
@@ -51,6 +51,7 @@
#include <net/ethernet.h>
#include <net/if.h>
+#include <net/if_gre.h>
#include <net/if_var.h>
#include <net/if_dl.h>
#include <net/if_types.h>
@@ -719,6 +719,19 @@
}
static void
+setifgrekey(const char *val, int dummy __unused, int s,
+ const struct afswtch *afp)
+{
+ uint32_t grekey = atol(val);
+
+ strncpy(ifr.ifr_name, name, sizeof (ifr.ifr_name));
+ ifr.ifr_data = (caddr_t)&grekey;
+ if (ioctl(s, GRESKEY, (caddr_t)&ifr) < 0)
+ warn("ioctl (set grekey)");
+}
+
+
+static void
setifname(const char *val, int dummy __unused, int s,
const struct afswtch *afp)
{
@@ -833,6 +846,12 @@
if (ioctl(s, SIOCGIFSTATUS, &ifs) == 0)
printf("%s", ifs.ascii);
+ int grekey = 0;
+ ifr.ifr_data = (caddr_t)&grekey;
+ if (ioctl(s, GREGKEY, &ifr) == 0)
+ if (grekey != 0)
+ printf("\tgrekey: %d\n", grekey);
+
close(s);
return;
}
@@ -989,6 +1008,7 @@
DEF_CMD("noicmp", IFF_LINK1, setifflags),
DEF_CMD_ARG("mtu", setifmtu),
DEF_CMD_ARG("name", setifname),
+ DEF_CMD_ARG("grekey", setifgrekey),
};
static __constructor void
diff -ur src/share/man/man4/gre.4 src.new/share/man/man4/gre.4
--- share/man/man4/gre.4 2006-10-19 10:41:47.000000000 +0300
+++ share/man/man4/gre.4 2007-07-19 02:11:28.909767302 +0300
@@ -167,6 +167,12 @@
section below.
.It Dv GREGPROTO
Query operation mode.
+.It Dv GRESKEY
+Set the GRE key used for outgoing packets (ifr_data must point to an uint32_t).
+A value of 0 disables the key option.
+.It Dv GREGKEY
+Get the GRE key currently used for outgoing packets (ifr_data must point to an
+uint32_t). 0 means no outgoing key.
.El
.Pp
Note that the IP addresses of the tunnel endpoints may be the same as the
@@ -263,7 +269,8 @@
.Sh NOTES
The MTU of
.Nm
-interfaces is set to 1476 by default, to match the value used by Cisco routers.
+interfaces is set to 1476 by default (except when setting grekey, in which
+case the MTU is 1472), so packets fit inside an Ethernet frame.
This may not be an optimal value, depending on the link between the two tunnel
endpoints.
It can be adjusted via
@@ -291,10 +298,6 @@
.Cm up
must be given last on its command line.
.Pp
-The kernel must be set to forward datagrams by setting the
-.Va net.inet.ip.forwarding
-.Xr sysctl 8
-variable to non-zero.
.Sh SEE ALSO
.\" Xr atalk 4 ,
.Xr gif 4 ,
@@ -332,4 +335,10 @@
.Nm
interface itself.
.Pp
-The GRE RFCs are not yet fully implemented (no GRE options).
+The current implementation uses the key only for outgoing packets.
+Incomming packets with a different key or without a key will be treated as if they
+would belong to this interface. There is currently little interest in implementing
+strict key checking because they are deprecated in RFC2784.
+.Pp
+RFC1701 is not fully supported, however all unsupported features have been
+deprecated in RFC2784.
diff -ur src/sys/net/if_gre.c src.new/sys/net/if_gre.c
--- sys/net/if_gre.c 2007-06-27 02:01:01.000000000 +0300
+++ sys/net/if_gre.c 2007-07-19 02:02:28.662034699 +0300
@@ -200,10 +200,11 @@
sc->g_proto = IPPROTO_GRE;
GRE2IFP(sc)->if_flags |= IFF_LINK0;
sc->encap = NULL;
- sc->called = 0;
sc->wccp_ver = WCCP_V1;
+ sc->key = 0;
if_attach(GRE2IFP(sc));
bpfattach(GRE2IFP(sc), DLT_NULL, sizeof(u_int32_t));
+
mtx_lock(&gre_mtx);
LIST_INSERT_HEAD(&gre_softc_list, sc, sc_list);
mtx_unlock(&gre_mtx);
@@ -247,18 +248,47 @@
u_int16_t etype = 0;
struct mobile_h mob_h;
u_int32_t af;
+ int gre_called;
+ struct m_tag *mtag;
/*
* gre may cause infinite recursion calls when misconfigured.
+ * We'll prevent this by detecting loops.
+ *
+ * High nesting level may cause stack exhaustion.
* We'll prevent this 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);
+ gre_called = 1;
+ mtag = m_tag_locate(m, MTAG_GRE, MTAG_GRE_CALLED, NULL);
+ while (mtag != NULL) {
+ if (*(struct ifnet **)(mtag + 1) == ifp) {
+ printf(
+ "gre_output: loop detected on %s\n",
+ (*(struct ifnet **)(mtag + 1))->if_xname);
+ m_freem(m);
+ error = EIO; /* is there better errno? */
+ goto end;
+ }
+ mtag = m_tag_locate(m, MTAG_GRE, MTAG_GRE_CALLED, mtag);
+ gre_called++;
+ }
+ if (gre_called > max_gre_nesting) {
+ printf(
+ "gre_output: recursively called too many times (%d)\n",
+ gre_called);
+ m_freem(m);
+ error = EIO; /* is there better errno? */
+ goto end;
+ }
+ mtag = m_tag_alloc(MTAG_GRE, MTAG_GRE_CALLED, sizeof(struct ifnet *),
+ M_NOWAIT);
+ if (mtag == NULL) {
m_freem(m);
- error = EIO; /* is there better errno? */
+ error = ENOMEM;
goto end;
}
+ *(struct ifnet **)(mtag + 1) = ifp;
+ m_tag_prepend(m, mtag);
if (!((ifp->if_flags & IFF_UP) &&
(ifp->if_drv_flags & IFF_DRV_RUNNING)) ||
@@ -381,7 +411,12 @@
error = EAFNOSUPPORT;
goto end;
}
- M_PREPEND(m, sizeof(struct greip), M_DONTWAIT);
+
+ /* Reserve space for GRE header + optional GRE key */
+ int hdrlen = sizeof(struct greip);
+ if (sc->key)
+ hdrlen += sizeof(uint32_t);
+ M_PREPEND(m, hdrlen, M_DONTWAIT);
} else {
_IF_DROP(&ifp->if_snd);
m_freem(m);
@@ -397,9 +432,18 @@
gh = mtod(m, struct greip *);
if (sc->g_proto == IPPROTO_GRE) {
- /* we don't have any GRE flags for now */
+ uint32_t *options = gh->gi_options;
+
memset((void *)gh, 0, sizeof(struct greip));
gh->gi_ptype = htons(etype);
+ gh->gi_flags = 0;
+
+ /* Add key option */
+ if (sc->key)
+ {
+ gh->gi_flags |= htons(GRE_KP);
+ *(options++) = htonl(sc->key);
+ }
}
gh->gi_pr = sc->g_proto;
@@ -424,7 +468,6 @@
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);
@@ -437,7 +480,6 @@
struct if_laddrreq *lifr = (struct if_laddrreq *)data;
struct in_aliasreq *aifr = (struct in_aliasreq *)data;
struct gre_softc *sc = ifp->if_softc;
- int s;
struct sockaddr_in si;
struct sockaddr *sa = NULL;
int error;
@@ -445,7 +487,6 @@
error = 0;
- s = splnet();
switch (cmd) {
case SIOCSIFADDR:
ifp->if_flags |= IFF_UP;
@@ -718,12 +759,22 @@
si.sin_addr.s_addr = sc->g_dst.s_addr;
bcopy(&si, &ifr->ifr_addr, sizeof(ifr->ifr_addr));
break;
+ case GRESKEY:
+ sc->key = fuword(ifr->ifr_data);
+ if (sc->key == 0)
+ ifp->if_mtu = GREMTU;
+ else
+ ifp->if_mtu = GREMTU - 4;
+ break;
+ case GREGKEY:
+ suword(ifr->ifr_data, sc->key);
+ break;
+
default:
error = EINVAL;
break;
}
- splx(s);
return (error);
}
diff -ur src/sys/net/if_gre.h src.new/sys/net/if_gre.h
--- sys/net/if_gre.h 2005-06-10 19:49:18.000000000 +0300
+++ sys/net/if_gre.h 2007-07-18 22:39:29.000000000 +0300
@@ -44,6 +44,10 @@
#ifdef _KERNEL
#include <sys/queue.h>
+/* mbuf_tags(9) for recursion prevention */
+#define MTAG_GRE 1184786929
+#define MTAG_GRE_CALLED 0
+
/*
* Version of the WCCP, need to be configured manually since
* header for version 2 is the same but IP payload is prepended
@@ -67,7 +71,8 @@
const struct encaptab *encap; /* encapsulation cookie */
- int called; /* infinite recursion preventer */
+ uint32_t key; /* key included in outgoing GRE packets */
+ /* zero means none */
wccp_ver_t wccp_ver; /* version of the WCCP */
};
@@ -78,6 +83,7 @@
u_int16_t flags; /* GRE flags */
u_int16_t ptype; /* protocol type of payload typically
Ether protocol type*/
+ uint32_t options[]; /* optional options */
/*
* from here on: fields are optional, presence indicated by flags
*
@@ -110,6 +116,7 @@
#define gi_dst gi_i.ip_dst
#define gi_ptype gi_g.ptype
#define gi_flags gi_g.flags
+#define gi_options gi_g.options
#define GRE_CP 0x8000 /* Checksum Present */
#define GRE_RP 0x4000 /* Routing Present */
@@ -174,6 +181,8 @@
#define GREGADDRD _IOWR('i', 104, struct ifreq)
#define GRESPROTO _IOW('i' , 105, struct ifreq)
#define GREGPROTO _IOWR('i', 106, struct ifreq)
+#define GREGKEY _IOWR('i', 107, struct ifreq)
+#define GRESKEY _IOW('i', 108, struct ifreq)
#ifdef _KERNEL
LIST_HEAD(gre_softc_head, gre_softc);
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200707182351.l6INp4aa066745>