Subject: Re: kern/29287: trap in fr_stlookup(): dangerous assumptions with mbuf chains
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Quentin Garnier <cube@cubidou.net>
List: netbsd-bugs
Date: 02/09/2005 07:25:49
--y2MHPAl/EzyWgzIZ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 08, 2005 at 01:43:01PM +0000, cube@cubidou.net wrote:
> >Number:         29287
> >Category:       kern
> >Synopsis:       trap in fr_stlookup(): dangerous assumptions with mbuf c=
hains
[...]
> >Description:
> 	I get a trap in fr_stlookup() about every day.  Until today I had no
> 	time to look at it closely, but now I have tracked the issue down to
> 	the way mbuf chains are passed to the IPFilter code.  It is not a bug
> 	in IPFilter itself.
>=20
> 	The two panics I could deeply analyze followed the same trace:
> 		o pptpctrl (from net/poptop) generates GRE packets from what
> 		  it receives from the matching ppp interface.
> 		o it doesn't write an IP header, so rip_output() generates
> 		  one.
> 		o doing so, it calls M_PREPEND on the mbuf chain to get space
> 		  for an IP header.
> 		o *sometimes* (obviously not always or I'd have noticed that
> 		  much earlier), the chain will get prepended with a mbuf of
> 		  size 20 whose data pointer gets "aligned" on a page.  What I
> 		  mean by that is the mbuf has space for 20 bytes, and then no
> 		  more, as it reach a page boundary.
> 		o rip_output transmits the mbuf chain to ip_output, and then
> 		  through the pfil hooks it reaches fr_check_wrapper.
> 		o fr_check_wrapper doens't check any length and passes the
> 		  data from the first mbuf of the chain to fr_check.
> 		o it reaches fr_stlookup, which has code for IPPROTO_GRE which
> 		  tries to access data in the GRE header, which it *assumes*
> 		  is right after the IP header.
> 		o kernel gets a page fault and my job security weakens.
>=20
> 	I think the trigger of the fault is the way the prepended packet is
> 	aligned.  If it is far enough from a page boundary, the code in
> 	fr_stlookup() will get garbage, but work, for some definition of it.
[...]
> >Fix:
> 	I'll try a gross hack for my server, doing a m_pullup(m, 36) in
> 	rip_output, but of course the right way to do that is to inform IPF of
> 	the size of the buffer it is working on.  Easier said than done, as
> 	the kernel would also have to provide a way for IPFilter to get the
> 	following chunks of data.

As Darren pointed out in a private mail, this has a good chance of being
fixed in -current after import of IPFilter 4.1.5, so I'll try with that
version of IPF.

--=20
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"When I find the controls, I'll go where I like, I'll know where I want
to be, but maybe for now I'll stay right here on a silent sea."
KT Tunstall, Silent Sea, Eye to the Telescope, 2004.

--y2MHPAl/EzyWgzIZ
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (NetBSD)

iQEVAwUBQgms7NgoQloHrPnoAQK60AgAgY6y+XxGF/9pv9Vd1vJiZ6cposO9Jx3Z
3bG4bxj1ZoDog19oFQSbnXHsE/fCt0j1KrhJ3dTETwEYMqqD+gbKpSejKIY+xdfW
632QGMUv77gfYWqGzht4LF4i6EbujgvJwOfeHkmAfxo033qyGAwVcVtVYCdt1S1M
wy5ObRzJZuqrtymqmkEerBewJyeqT2LUvkYJN+C7N4Vu1ApV3zjCdVsgSILwNH8I
zqrwCRP3HocC73EzLmHDDkxMfQM7GmMB08NYrhMYpCP7c4lXGItuLcmyCihNXhN6
nh8Pn3h1Ufe3Vpx/tciFy/w2f+xsyfPQvFTV3PI/7E9YqI25g7SDBw==
=BK31
-----END PGP SIGNATURE-----

--y2MHPAl/EzyWgzIZ--