IPv4: ip_options_compile() how can we avoid blowing up on a NULL skb???

2006-11-16 Thread Jesper Juhl 

Hi,

In net/ipv4/ip_options.c::ip_options_compile() we have the following
code at the start of the function :


int ip_options_compile(struct ip_options * opt, struct sk_buff * skb)
{
   int l;
   unsigned char * iph;
   unsigned char * optptr;
   int optlen;
   unsigned char * pp_ptr = NULL;
   struct rtable *rt = skb ? (struct rtable*)skb-dst : NULL;

   if (!opt) {
   opt = (IPCB(skb)-opt);
   iph = skb-nh.raw;
   opt-optlen = ((struct iphdr *)iph)-ihl*4 -
sizeof(struct iphdr);
   optptr = iph + sizeof(struct iphdr);
   opt-is_data = 0;
   } else {
   optptr = opt-is_data ? opt-__data : (unsigned
char*)(skb-nh.iph[1]);
   iph = optptr - sizeof(struct iphdr);
   }

...

I don't see how that avoids blowing up if we get passed a NULL skb.


From the line

   struct rtable *rt = skb ? (struct rtable*)skb-dst : NULL;
it is clear that we /may/ get passed a null skb.

Then a bit further down in the  if (!opt)  bit we dereference 'skb' :
   opt = (IPCB(skb)-opt);
and we also may dereference it in the  else  part :
   optptr = opt-is_data ? opt-__data : (unsigned char*)(skb-nh.iph[1]);

So if 'skb' is NULL, the only route I see that doesn't cause a NULL
pointer deref is if  (opt != NULL)  and at the same time
(opt-is_data != NULL)  .   Is that guaranteed in any way?  If now,
how come we don't blow up regularly?


--
Jesper Juhl [EMAIL PROTECTED]
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: IPv4: ip_options_compile() how can we avoid blowing up on a NULL skb???

2006-11-16 Thread David Miller 
From: Jesper Juhl [EMAIL PROTECTED]
Date: Thu, 16 Nov 2006 23:34:14 +0100

 So if 'skb' is NULL, the only route I see that doesn't cause a NULL
 pointer deref is if  (opt != NULL)  and at the same time
 (opt-is_data != NULL)  .   Is that guaranteed in any way?

Yes, it is guarenteed, all callers  make sure these invariants
hold true.

I'm very happy to accept a patch which assert's this using BUG()
checks :-)
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: IPv4: ip_options_compile() how can we avoid blowing up on a NULL skb???

2006-11-16 Thread Herbert Xu 
Jesper Juhl [EMAIL PROTECTED] wrote:
 
 So if 'skb' is NULL, the only route I see that doesn't cause a NULL
 pointer deref is if  (opt != NULL)  and at the same time
 (opt-is_data != NULL)  .   Is that guaranteed in any way?  If now,
 how come we don't blow up regularly?

Yes that's how it's supposed to be used.  This function either constructs
an opts structure from a packet, or it verifies the validity of a suspect
opts structure (without a packet).  In the latter case, both opt and
opt-is_data should be non-zero.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: IPv4: ip_options_compile() how can we avoid blowing up on a NULL skb???

2006-11-16 Thread Herbert Xu 
David Miller [EMAIL PROTECTED] wrote:

 I'm very happy to accept a patch which assert's this using BUG()
 checks :-)

A BUG() won't be necessary because the NULL pointer dereferences will
OOPS anyway.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: IPv4: ip_options_compile() how can we avoid blowing up on a NULL skb???

2006-11-16 Thread Herbert Xu 
Dmitry Torokhov [EMAIL PROTECTED] wrote:
 
 BUG()s there would be a mechanism to document invariants so next time
 someone is looking at the code there are no questions.

Well if someone is documenting this then wouldn't a comment (or even a
kerneldoc style block) be better?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html