[PATCH] ipx: header length validation needed

[PATCH] ipx: header length validation needed

[Stephen Hemminger]

IPX is not checking for non-linear (and short packets) in it's receive routine.
This is serious because it may mean it ends up reading past end of skb.

This maybe related to this bug, because sky2 will copy small packets into small
skb's.

    http://bugzilla.kernel.org/show_bug.cgi?id=6693

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>


--- a/net/ipx/af_ipx.c	2006-07-07 13:02:31.000000000 -0700
+++ b/net/ipx/af_ipx.c	2006-08-07 13:18:08.000000000 -0700
@@ -1642,6 +1642,9 @@
 	if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL)
 		goto out;
 
+	if (!pskb_may_pull(skb, sizeof(struct ipxhdr)))
+		goto drop;
+
 	ipx		= ipx_hdr(skb);
 	ipx_pktsize	= ntohs(ipx->ipx_pktsize);
 	

Re: [PATCH] ipx: header length validation needed

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Mon, 7 Aug 2006 13:46:36 -0700

> IPX is not checking for non-linear (and short packets) in it's
> receive routine.  This is serious because it may mean it ends up
> reading past end of skb.

This takes care of ipx_rcv() but the rest of the IPX protocol
handling still has the problem, so you'll need to meticuliously
follow the whole receive path and fix up all the spots that
parse subsequent parts of the IPX packet to fix this properly.

For example, take a look at ipxitf_pprop().

Re: [PATCH] ipx: header length validation needed

[Stephen Hemminger]

This patch will linearize and check there is enough data.
It handles the pprop case as well as avoiding a whole audit of
the routing code.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

--- a/net/ipx/af_ipx.c	2006-08-07 13:45:59.000000000 -0700
+++ b/net/ipx/af_ipx.c	2006-08-07 16:34:00.000000000 -0700
@@ -1649,7 +1649,8 @@
 	ipx_pktsize	= ntohs(ipx->ipx_pktsize);
 	
 	/* Too small or invalid header? */
-	if (ipx_pktsize < sizeof(struct ipxhdr) || ipx_pktsize > skb->len)
+	if (ipx_pktsize < sizeof(struct ipxhdr)
+	   || !pskb_may_pull(skb, ipx_pktsize))
 		goto drop;
                         
 	if (ipx->ipx_checksum != IPX_NO_CHECKSUM &&

Re: [PATCH] ipx: header length validation needed

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Mon, 7 Aug 2006 16:36:02 -0700

> This patch will linearize and check there is enough data.
> It handles the pprop case as well as avoiding a whole audit of
> the routing code.
> 
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

Applied, with a minor coding style tidy-up.

Thanks a lot Stephen.

Re: [stable] [PATCH] ipx: header length validation needed

[Greg KH]

On Mon, Aug 07, 2006 at 04:36:02PM -0700, Stephen Hemminger wrote:
> This patch will linearize and check there is enough data.
> It handles the pprop case as well as avoiding a whole audit of
> the routing code.
> 
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

queued to -stable, thanks.

greg k-h