Date: Wed, 31 Mar 2004 01:03:49 +0300 From: Ruslan Ermilov <ru@FreeBSD.org> To: Robert Watson <rwatson@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/net if_gif.c Message-ID: <20040330220349.GA97921@ip.net.ua> In-Reply-To: <200403221424.i2MEOQYK057524@repoman.freebsd.org> References: <200403221424.i2MEOQYK057524@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--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/--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040330220349.GA97921>