From owner-cvs-src@FreeBSD.ORG  Tue Mar 30 14:03:45 2004
Return-Path: <owner-cvs-src@FreeBSD.ORG>
Delivered-To: cvs-src@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP
	id DAD9C16A4CE; Tue, 30 Mar 2004 14:03:45 -0800 (PST)
Received: from tigra.ip.net.ua (tigra.ip.net.ua [82.193.96.10])
	by mx1.FreeBSD.org (Postfix) with ESMTP
	id BAE6A43D1F; Tue, 30 Mar 2004 14:03:44 -0800 (PST)
	(envelope-from ru@ip.net.ua)
Received: from heffalump.ip.net.ua (heffalump.ip.net.ua [82.193.96.213])
	by tigra.ip.net.ua (8.12.11/8.12.11) with ESMTP id i2UM7H4Z025327
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO);
	Wed, 31 Mar 2004 01:07:18 +0300 (EEST)
	(envelope-from ru@ip.net.ua)
Received: (from ru@localhost)
	by heffalump.ip.net.ua (8.12.11/8.12.11) id i2UM3nSq098006;
	Wed, 31 Mar 2004 01:03:49 +0300 (EEST)
	(envelope-from ru)
Date: Wed, 31 Mar 2004 01:03:49 +0300
From: Ruslan Ermilov <ru@FreeBSD.org>
To: Robert Watson <rwatson@FreeBSD.org>
Message-ID: <20040330220349.GA97921@ip.net.ua>
References: <200403221424.i2MEOQYK057524@repoman.freebsd.org>
Mime-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="61jdw2sOBCFtR2d/"
Content-Disposition: inline
In-Reply-To: <200403221424.i2MEOQYK057524@repoman.freebsd.org>
User-Agent: Mutt/1.5.6i
X-Spam-Checker-Version: SpamAssassin 2.55 (1.174.2.19-2003-05-19-exp)
cc: cvs-src@FreeBSD.org
cc: src-committers@FreeBSD.org
cc: cvs-all@FreeBSD.org
Subject: Re: cvs commit: src/sys/net if_gif.c
X-BeenThere: cvs-src@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
List-Id: CVS commit messages for the src tree <cvs-src.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/cvs-src>,
	<mailto:cvs-src-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/cvs-src>
List-Post: <mailto:cvs-src@freebsd.org>
List-Help: <mailto:cvs-src-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/cvs-src>,
	<mailto:cvs-src-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 30 Mar 2004 22:03:46 -0000


--61jdw2sOBCFtR2d/
Content-Type: multipart/mixed; boundary="EVF5PPMfhYS0aIcm"
Content-Disposition: inline


--EVF5PPMfhYS0aIcm
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Mar 22, 2004 at 06:24:26AM -0800, Robert Watson wrote:
> rwatson     2004/03/22 06:24:26 PST
>=20
>   FreeBSD src repository
>=20
>   Modified files:
>     sys/net              if_gif.c=20
>   Log:
>   Move "called", a static function variable used to detect recursive
>   processing with gif interfaces, to a global variable named "gif_called".
>   Add an annotation that this approach will not work with a reentrant
>   network stack, and that we should instead use packet tags to detect
>   excessive recursive processing.
>  =20
>   Revision  Changes    Path
>   1.42      +11 -4     src/sys/net/if_gif.c
>=20
Implemented this in the attached patch.  Note when testing: setting
net.link.gif.max_nesting too high (>20 on my system) and triggering
the recursion causes the kernel stack exhaustion.


Cheers,
--=20
Ruslan Ermilov
ru@FreeBSD.org
FreeBSD committer

--EVF5PPMfhYS0aIcm
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=p
Content-Transfer-Encoding: quoted-printable

Index: if_gif.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /home/ncvs/src/sys/net/if_gif.c,v
retrieving revision 1.43
diff -u -p -r1.43 if_gif.c
--- if_gif.c	22 Mar 2004 15:43:14 -0000	1.43
+++ if_gif.c	30 Mar 2004 21:57:42 -0000
@@ -84,22 +84,13 @@
 #define GIFNAME		"gif"
=20
 /*
- * gif_mtx protects the global gif_softc_list, and access to gif_called.
- * XXX: See comment blow on gif_called.
+ * gif_mtx protects the global gif_softc_list.
  * XXX: Per-softc locking is still required.
  */
 static struct mtx gif_mtx;
 static MALLOC_DEFINE(M_GIF, "gif", "Generic Tunnel Interface");
 static LIST_HEAD(, gif_softc) gif_softc_list;
=20
-/*
- * XXX: gif_called is a recursion counter to prevent misconfiguration to
- * cause unbounded looping in the network stack.  However, this is a flawed
- * approach as it assumes non-reentrance in the stack.  This should be
- * changed to use packet tags to track recusion..
- */
-static int gif_called =3D 0;
-
 void	(*ng_gif_input_p)(struct ifnet *ifp, struct mbuf **mp, int af);
 void	(*ng_gif_input_orphan_p)(struct ifnet *ifp, struct mbuf *m, int af);
 void	(*ng_gif_attach_p)(struct ifnet *ifp);
@@ -347,6 +338,7 @@ gif_output(ifp, m, dst, rt)
 	struct rtentry *rt;	/* added in net2 */
 {
 	struct gif_softc *sc =3D (struct gif_softc*)ifp;
+	struct m_tag *mtag;
 	int error =3D 0;
=20
 #ifdef MAC
@@ -360,21 +352,26 @@ gif_output(ifp, m, dst, rt)
 	/*
 	 * gif may cause infinite recursion calls when misconfigured.
 	 * We'll prevent this by introducing upper limit.
-	 * XXX: this mechanism may introduce another problem about
-	 *      mutual exclusion of the variable CALLED, especially if we
-	 *      use kernel thread.
 	 */
-	mtx_lock(&gif_mtx);
-	if (++gif_called > max_gif_nesting) {
-		mtx_unlock(&gif_mtx);
+	mtag =3D m_tag_locate(m, MTAG_GIF, MTAG_GIF_CALLED, NULL);
+	if (mtag =3D=3D NULL) {
+		mtag =3D m_tag_alloc(MTAG_GIF, MTAG_GIF_CALLED,
+		    sizeof(int), M_NOWAIT);
+		if (mtag =3D=3D NULL) {
+			m_freem(m);
+			error =3D ENOMEM;
+			goto end;
+		}
+		*(int *)(mtag + 1) =3D 0;
+		m_tag_prepend(m, mtag);
+	} else if (++*(int *)(mtag + 1) > max_gif_nesting) {
 		log(LOG_NOTICE,
 		    "gif_output: recursively called too many times(%d)\n",
-		    gif_called);
+		    *(int *)(mtag + 1));
 		m_freem(m);
 		error =3D EIO;	/* is there better errno? */
 		goto end;
 	}
-	mtx_unlock(&gif_mtx);
=20
 	m->m_flags &=3D ~(M_BCAST|M_MCAST);
 	if (!(ifp->if_flags & IFF_UP) ||
@@ -414,9 +411,6 @@ gif_output(ifp, m, dst, rt)
 	}
=20
   end:
-	mtx_lock(&gif_mtx);
-	gif_called =3D 0;		/* reset recursion counter */
-	mtx_unlock(&gif_mtx);
 	if (error)
 		ifp->if_oerrors++;
 	return error;
Index: if_gif.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /home/ncvs/src/sys/net/if_gif.h,v
retrieving revision 1.14
diff -u -p -r1.14 if_gif.h
--- if_gif.h	16 Oct 2002 19:49:37 -0000	1.14
+++ if_gif.h	30 Mar 2004 20:49:31 -0000
@@ -81,6 +81,9 @@ struct gif_softc {
 #define	GIF_MTU_MIN	(1280)	/* Minimum MTU */
 #define	GIF_MTU_MAX	(8192)	/* Maximum MTU */
=20
+#define	MTAG_GIF	1080679712
+#define	MTAG_GIF_CALLED	0
+
 /* Prototypes */
 void gifattach0(struct gif_softc *);
 void gif_input(struct mbuf *, int, struct ifnet *);

--EVF5PPMfhYS0aIcm--

--61jdw2sOBCFtR2d/
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (FreeBSD)

iD8DBQFAae7FUkv4P6juNwoRAmK0AJ4hzzJAvoYhAZRF7L2Of5B3NMRr+QCghWTg
dI5UQNuGvYHPtqmy4DNo0Zg=
=ztvf
-----END PGP SIGNATURE-----

--61jdw2sOBCFtR2d/--