Re: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-25 Thread David S. Miller
From: Herbert Xu [EMAIL PROTECTED]
Date: Sat, 25 Mar 2006 08:45:39 +1100

 Hugo Santos [EMAIL PROTECTED] wrote:
This patch fixes a soft lockup in ip6_tunnel when not using
  xfrm6_tunnel (CONFIG_INET6_TUNNEL). It is triggered when an encapsula-
  ted packet reaches ip6ip6_rcv() and there is no tunnel associated with
  it. The error path returns a positive value (1) which will trigger
  ip6_input to re-submit the packet for processing. As no skb parameters
  have been changed, ip6ip6_rcv() will continue to be called with the
  exact same context. Also, ip6ip6_rcv() should free the skb when
  discarding it.
  
  Signed-off-by: Hugo Santos [EMAIL PROTECTED]
 
 OK this is a bit ugly but will do for now.  Could you please do it for
 the ICMP packet and IPv4 as well?

I don't want to apply this, for now, it's correct but really ugly :)

I'd rather the suggested cleanup occur to solve this, and I think
the fix is not so urgent that we can wait for the correct version
to get coded up.

Thanks.
-
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: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-25 Thread Hugo Santos
 I'd rather the suggested cleanup occur to solve this, and I think
 the fix is not so urgent that we can wait for the correct version
 to get coded up.

   I would be glad to code a better version like i specified in an
 earlier mail. I just didn't do it yet because Herbert said he would do
 it.  And Dave, i'm not sure i agree with it not being urgent. Surely
 the number of people that have ip6_tunnel loaded is reduced, but any
 host in the internet is able to hang any such machine by sending an
 easily crafted packet. If Herbert doesn't have the time, i can code the
 patch today as this fix is important for me.

   Hugo


signature.asc
Description: Digital signature


Re: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-25 Thread Hugo Santos
  host in the internet is able to hang any such machine by sending an

   The ipv6-enabled internet of course :-)

   Hugo


signature.asc
Description: Digital signature


Re: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-24 Thread Hugo Santos
 When xfrm6_tunnel is in use you've just made it use a freed skb.  Also
 IPv4 has the same problem so we shold fix them both.

   I didn't hit this since i'm not currently using xfrm6_tunnel (which
 is also why i got the soft lockup). I'll consider the case when
 xfrm6_tunnel is being used, and submit another patch.

   Thanks,
  Hugo


signature.asc
Description: Digital signature


Re: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-24 Thread Herbert Xu
Hugo Santos [EMAIL PROTECTED] wrote:
 
   This patch fixes a soft lockup in ip6_tunnel when not using
 xfrm6_tunnel (CONFIG_INET6_TUNNEL). It is triggered when an encapsula-
 ted packet reaches ip6ip6_rcv() and there is no tunnel associated with
 it. The error path returns a positive value (1) which will trigger
 ip6_input to re-submit the packet for processing. As no skb parameters
 have been changed, ip6ip6_rcv() will continue to be called with the
 exact same context. Also, ip6ip6_rcv() should free the skb when
 discarding it.
 
 Signed-off-by: Hugo Santos [EMAIL PROTECTED]

OK this is a bit ugly but will do for now.  Could you please do it for
the ICMP packet and IPv4 as well?

Thanks,
-- 
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: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-24 Thread Hugo Santos
 OK this is a bit ugly but will do for now.  Could you please do it for
 the ICMP packet and IPv4 as well?

   I find it ugly that the same function, in this case ip6ip6_rcv(), is
 used in two contexts where the expected behaviour differs. Also, using
 the current #ifdefs, ip6_tunnel is only loadable when xfrm6_tunnel
 isn't loaded (as xfrm6_tunnel takes over as IPPROTO_IPV6 handler). A
 solution would involve ip6_tunnel depend on xfrm6_tunnel if it was
 built either statically or as a module (something that i don't quite
 appreciate due to the extra tunnel-spi-lookup but that could do for
 now).

   Hugo

 
 Thanks,
 -- 
 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


signature.asc
Description: Digital signature


Re: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-24 Thread David S. Miller
From: Hugo Santos [EMAIL PROTECTED]
Date: Sat, 25 Mar 2006 00:18:04 +

I find it ugly that the same function, in this case ip6ip6_rcv(), is
  used in two contexts where the expected behaviour differs.

It would have been even worse, IMHO, to have two copies of
nearly identical code sitting around which is basically what
the alternative is.
-
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: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-24 Thread Herbert Xu
On Fri, Mar 24, 2006 at 04:31:15PM -0800, David S. Miller wrote:
 
 It would have been even worse, IMHO, to have two copies of
 nearly identical code sitting around which is basically what
 the alternative is.

Actually, it might be cleaner to have wrappers around a common
(possibly inline as well) function.  Let me have a go at that.

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: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-24 Thread David S. Miller
From: Herbert Xu [EMAIL PROTECTED]
Date: Sat, 25 Mar 2006 11:36:36 +1100

 On Fri, Mar 24, 2006 at 04:31:15PM -0800, David S. Miller wrote:
  
  It would have been even worse, IMHO, to have two copies of
  nearly identical code sitting around which is basically what
  the alternative is.
 
 Actually, it might be cleaner to have wrappers around a common
 (possibly inline as well) function.  Let me have a go at that.

Sounds good.
-
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: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-24 Thread Hugo Santos
 It would have been even worse, IMHO, to have two copies of
 nearly identical code sitting around which is basically what
 the alternative is.

   We don't need two copies. We just need a function that does the real
 work if there is a tunnel to be used (what xfrm6_tunnel needs), and
 another one that would call the previous (used when xfrm6_tunnel isn't
 used) that frees the skb on errors. Something like:

   int ip6ip6_rcv(...) {
 ...
   }

   int ip6ip6_proto_rcv(...) {
 int res = ip6ip6_rcv();
 if (res  0) {
icmpv6_send_error();
kfree_skb();
 }
 return res;
   }

   Where ip6ip6_rcv is used when ip6_tunnel is registed via
 xfrm6_tunnel, and the second as the IPPROTO_IPV6 handler (when this
 functionality is compiled in).

   Hugo

 -
 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


signature.asc
Description: Digital signature


Re: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-23 Thread YOSHIFUJI Hideaki / 吉藤英明
In article [EMAIL PROTECTED] (at Thu, 23 Mar 2006 16:34:12 +), Hugo 
Santos [EMAIL PROTECTED] says:

This patch fixes a soft lockup when encapsulated packets reach
  ip6ip6_rcv() and there is no tunnel associated with it. The error
  path returns a positive value (1) which will trigger ip6_input to
  re-submit the packet for processing. As no skb parameters have been
  changed, ip6ip6_rcv() will continue to be called on the exact same
  context. Also, ip6ip6_rcv() should free the skb when discarding it.
 
  Signed-off-by: Hugo Santos [EMAIL PROTECTED]
Acked-by: YOSHIFUJI Hideaki [EMAIL PROTECTED]

-- 
YOSHIFUJI Hideaki @ USAGI Project  [EMAIL PROTECTED]
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA
-
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: [PATCH] ip6_tunnel: fix a soft lockup when there is no active tunnel for an encapsulated packet

2006-03-23 Thread Herbert Xu
Hugo Santos [EMAIL PROTECTED] wrote:
 
   This patch fixes a soft lockup when encapsulated packets reach
 ip6ip6_rcv() and there is no tunnel associated with it. The error
 path returns a positive value (1) which will trigger ip6_input to
 re-submit the packet for processing. As no skb parameters have been
 changed, ip6ip6_rcv() will continue to be called on the exact same
 context. Also, ip6ip6_rcv() should free the skb when discarding it.

The assessment is correct but I think this fix is wrong.
 
 --- linux-2.6.16/net/ipv6/ip6_tunnel.c.orig 2006-03-23 16:19:19.0 
 +
 +++ linux-2.6.16/net/ipv6/ip6_tunnel.c  2006-03-23 16:32:07.0 +
 @@ -557,7 +557,8 @@ ip6ip6_rcv(struct sk_buff **pskb)
read_unlock(ip6ip6_lock);
icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_ADDR_UNREACH, 0, 
 skb-dev);
 discard:
 -   return 1;
 +   kfree_skb(skb);
 +   return -1;
 }

When xfrm6_tunnel is in use you've just made it use a freed skb.  Also
IPv4 has the same problem so we shold fix them both.

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