Re: IPV6 source routing patch is still broken?

2007-04-27 Thread David Miller 
From: Chuck Ebbert [EMAIL PROTECTED]
Date: Thu, 26 Apr 2007 18:57:06 -0400

 David Miller wrote:
  +   case IPV6_SRCRT_TYPE_2:
  +   if (accept_source_route = 0)
  +   break;
  +   kfree_skb(skb);
  +   return -1;
  +   case IPV6_SRCRT_TYPE_0:
  +   if (accept_source_route  0)
  +   break;
  +   kfree_skb(skb);
  +   return -1;
  
  Yes, that looks like it matches the sysctl documentation more closely:
  
  accept_source_route - INTEGER
  Accept source routing (routing extension header).
  
   0: Accept routing header.
  = 0: Accept only routing header type 2.
   0: Do not accept routing header.
  
  Type 2 packets should get through as long as the value of the sysctl
  is not negative.
 
 It was Sergey Vlasov who first found this. I had tried to find his original
 message but I was searching the wrong place.

Actually, earlier in the function accept_source_route is
verified, and if it is negative ipv6_rthdr_rcv() returns
immediately.  This is done by the initial code which reads:

if (accept_source_route  0 ||
((idev = in6_dev_get(skb-dev)) == NULL)) {
kfree_skb(skb);
return -1;
}
if (idev-cnf.accept_source_route  0) {
in6_dev_put(idev);
kfree_skb(skb);
return -1;
}

then the function proceeds to use the largest of
'accept_source_route' and 'idev-cnf.accept_source_route'
for further checks.

So when we get to the switch statement in question, we know
it will be a positive value, so none of the purely negative
cases need to be considered.

So with Yoshifuji-sans small fix, the switch statement
covers all the cases properly:

switch (hdr-type) {
#ifdef CONFIG_IPV6_MIP6
case IPV6_SRCRT_TYPE_2:
break;
#endif
case IPV6_SRCRT_TYPE_0:
if (accept_source_route  0)
break;
kfree_skb(skb);
return -1;
default:
IP6_INC_STATS_BH(ip6_dst_idev(skb-dst),
 IPSTATS_MIB_INHDRERRORS);
icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, (hdr-type) - 
skb-nh.raw);
return -1;
}
-
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: IPV6 source routing patch is still broken?

2007-04-27 Thread YOSHIFUJI Hideaki / 吉藤英明 
In article [EMAIL PROTECTED] (at Fri, 27 Apr 2007 02:08:05 -0700 (PDT)), 
David Miller [EMAIL PROTECTED] says:

 then the function proceeds to use the largest of
 'accept_source_route' and 'idev-cnf.accept_source_route'
 for further checks.

Smallest:
if (accept_source_route  idev-cnf.accept_source_route)
accept_source_route = idev-cnf.accept_source_route;

--yoshufji
-
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: IPV6 source routing patch is still broken?

2007-04-27 Thread David Miller 
From: YOSHIFUJI Hideaki / 吉藤英明 [EMAIL PROTECTED]
Date: Fri, 27 Apr 2007 18:36:35 +0900 (JST)

 In article [EMAIL PROTECTED] (at Fri, 27 Apr 2007 02:08:05 -0700 (PDT)), 
 David Miller [EMAIL PROTECTED] says:
 
  then the function proceeds to use the largest of
  'accept_source_route' and 'idev-cnf.accept_source_route'
  for further checks.
 
 Smallest:
 if (accept_source_route  idev-cnf.accept_source_route)
 accept_source_route = idev-cnf.accept_source_route;

Correct, my mistake.
-
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

IPV6 source routing patch is still broken?

2007-04-26 Thread Chuck Ebbert 
Looking at the patch that went into 2.6.20.9, I can't see
how type 2 packets get through at all. Shouldn't this part
read:

+   case IPV6_SRCRT_TYPE_2:
+   if (accept_source_route = 0)
+   break;
+   kfree_skb(skb);
+   return -1;
+   case IPV6_SRCRT_TYPE_0:
+   if (accept_source_route  0)
+   break;
+   kfree_skb(skb);
+   return -1;

And what does this do?

+   switch (hdr-type) {
+#ifdef CONFIG_IPV6_MIP6
+   break;
+#endif

break by itself like that doesn't do anything. If it did
then people with mobile ipv6 enabled would always accept all
source routing. (They should at least know that's a problem.)

-
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: IPV6 source routing patch is still broken?

2007-04-26 Thread David Miller 
From: Chuck Ebbert [EMAIL PROTECTED]
Date: Thu, 26 Apr 2007 17:53:58 -0400

 And what does this do?
 
 + switch (hdr-type) {
 +#ifdef CONFIG_IPV6_MIP6
 + break;
 +#endif
 
 break by itself like that doesn't do anything. If it did
 then people with mobile ipv6 enabled would always accept all
 source routing. (They should at least know that's a problem.)

We are aware of this part already, and I've sent the fix to
the -stable folks and Linus so we can figure out how to deal
with this and the netlink OOPS'er too.

To be honest I don't blame anyone but the compiler, it shouldn't
accept that code without at least a warning.
-
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: IPV6 source routing patch is still broken?

2007-04-26 Thread David Miller 
From: Chuck Ebbert [EMAIL PROTECTED]
Date: Thu, 26 Apr 2007 17:53:58 -0400

 Looking at the patch that went into 2.6.20.9, I can't see
 how type 2 packets get through at all. Shouldn't this part
 read:
 
 +   case IPV6_SRCRT_TYPE_2:
 +   if (accept_source_route = 0)
 +   break;
 +   kfree_skb(skb);
 +   return -1;
 +   case IPV6_SRCRT_TYPE_0:
 +   if (accept_source_route  0)
 +   break;
 +   kfree_skb(skb);
 +   return -1;

Yes, that looks like it matches the sysctl documentation more closely:

accept_source_route - INTEGER
Accept source routing (routing extension header).

 0: Accept routing header.
= 0: Accept only routing header type 2.
 0: Do not accept routing header.

Type 2 packets should get through as long as the value of the sysctl
is not negative.

Hmmm?
-
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: IPV6 source routing patch is still broken?

2007-04-26 Thread Chuck Ebbert 
David Miller wrote:
 +   case IPV6_SRCRT_TYPE_2:
 +   if (accept_source_route = 0)
 +   break;
 +   kfree_skb(skb);
 +   return -1;
 +   case IPV6_SRCRT_TYPE_0:
 +   if (accept_source_route  0)
 +   break;
 +   kfree_skb(skb);
 +   return -1;
 
 Yes, that looks like it matches the sysctl documentation more closely:
 
 accept_source_route - INTEGER
   Accept source routing (routing extension header).
 
0: Accept routing header.
   = 0: Accept only routing header type 2.
0: Do not accept routing header.
 
 Type 2 packets should get through as long as the value of the sysctl
 is not negative.

It was Sergey Vlasov who first found this. I had tried to find his original
message but I was searching the wrong place.

Sergey, you should send networking-related messages to netdev.

-
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: IPV6 source routing patch is still broken?

2007-04-26 Thread Chuck Ebbert 
David Miller wrote:
 From: Chuck Ebbert [EMAIL PROTECTED]
 Date: Thu, 26 Apr 2007 17:53:58 -0400
 
 And what does this do?

 +switch (hdr-type) {
 +#ifdef CONFIG_IPV6_MIP6
 +break;
 +#endif

 break by itself like that doesn't do anything. If it did
 then people with mobile ipv6 enabled would always accept all
 source routing. (They should at least know that's a problem.)
 
 We are aware of this part already, and I've sent the fix to
 the -stable folks and Linus so we can figure out how to deal
 with this and the netlink OOPS'er too.
 

Another question: is it a good idea to even be shipping release
kernels with Mobile IPV6 enabled? Unfortunately, experimental
isn't enough to go by...

-
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: IPV6 source routing patch is still broken?

2007-04-26 Thread YOSHIFUJI Hideaki / 吉藤英明 
In article [EMAIL PROTECTED] (at Thu, 26 Apr 2007 19:10:56 -0400), Chuck 
Ebbert [EMAIL PROTECTED] says:

 Another question: is it a good idea to even be shipping release
 kernels with Mobile IPV6 enabled? Unfortunately, experimental
 isn't enough to go by...

Well, we have still 2 missing pieces in 2.6.21, and we are trying to
push them (source address selection, which is already in net-2.6 and
xfrm subpolicy, which is still on dave's backlog?) to 2.6.22.
After 2.6.23, we would say yes.

--yoshfuji
-
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: IPV6 source routing patch is still broken?

2007-04-26 Thread David Miller 
From: Chuck Ebbert [EMAIL PROTECTED]
Date: Thu, 26 Apr 2007 19:10:56 -0400

 Another question: is it a good idea to even be shipping release
 kernels with Mobile IPV6 enabled? Unfortunately, experimental
 isn't enough to go by...

There are no known issues in the mobile-ipv6 code to
my knowledge.  At least no OOPS'ers or anything like
that.
-
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