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/--