netdev
[Top] [All Lists]

Re: More IPSEC trouble

To: Steve Hill <steve@xxxxxxxxxxxxxxxxxxx>
Subject: Re: More IPSEC trouble
From: Patrick McHardy <kaber@xxxxxxxxx>
Date: Sat, 12 Mar 2005 01:13:23 +0100
Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>, netdev@xxxxxxxxxxx, "David S. Miller" <davem@xxxxxxxxxxxxx>
In-reply-to: <423233B9.50204@xxxxxxxxx>
References: <E1D9Ymg-0001x0-00@xxxxxxxxxxxxxxxxxxxxxxxx> <Pine.LNX.4.61.0503111453320.18067@xxxxxxxxxxxxxx> <42323125.20706@xxxxxxxxx> <423233B9.50204@xxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.5) Gecko/20050106 Debian/1.7.5-1
Patrick McHardy wrote:
Patrick McHardy wrote:

Steve Hill wrote:

This was a configuration mistake on my part and admittedly it shouldn't work properly - however, it triggered a kernel bug: sending a packet with the DF flag set which will grow to be > the MTU when encrypted causes the kernel to generate an ICMP Frag Needed packet, which got caught by the policy and this triggered the kernel to lock up hard.



Thanks for tracking this down, we need to unlock the state before
calling icmp_send(). This patch fixes it, it should apply to 2.6.10
if you replace dst_mtu() by dst_pmtu() in the context.


Second try .. this one compiles.

Embarrasing .. had I actually attached the new patch it would
have compiled :) Time to go to bed it seems ..

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2005/03/12 01:10:40+01:00 kaber@xxxxxxxxxxxx 
#   [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#   
#   Signed-off-by: Patrick McHardy <kaber@xxxxxxxxx>
# 
# net/ipv6/xfrm6_output.c
#   2005/03/12 01:10:31+01:00 kaber@xxxxxxxxxxxx +5 -3
#   [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#   
#   Signed-off-by: Patrick McHardy <kaber@xxxxxxxxx>
# 
# net/ipv4/xfrm4_output.c
#   2005/03/12 01:10:31+01:00 kaber@xxxxxxxxxxxx +5 -3
#   [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#   
#   Signed-off-by: Patrick McHardy <kaber@xxxxxxxxx>
# 
diff -Nru a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
--- a/net/ipv4/xfrm4_output.c   2005-03-12 01:12:12 +01:00
+++ b/net/ipv4/xfrm4_output.c   2005-03-12 01:12:12 +01:00
@@ -67,7 +67,7 @@
        memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
 }
 
-static int xfrm4_tunnel_check_size(struct sk_buff *skb)
+static int xfrm4_tunnel_check_size(struct xfrm_state *x, struct sk_buff *skb)
 {
        int mtu, ret = 0;
        struct dst_entry *dst;
@@ -84,6 +84,7 @@
        dst = skb->dst;
        mtu = dst_mtu(dst);
        if (skb->len > mtu) {
+               spin_unlock_bh(&x->lock);
                icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED, htonl(mtu));
                ret = -EMSGSIZE;
        }
@@ -109,9 +110,10 @@
                goto error;
 
        if (x->props.mode) {
-               err = xfrm4_tunnel_check_size(skb);
+               err = xfrm4_tunnel_check_size(x, skb);
                if (err)
-                       goto error;
+                       /* xfrm4_tunnel_check_size() drops the lock on error */
+                       goto error_nolock;
        }
 
        xfrm4_encap(skb);
diff -Nru a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
--- a/net/ipv6/xfrm6_output.c   2005-03-12 01:12:12 +01:00
+++ b/net/ipv6/xfrm6_output.c   2005-03-12 01:12:12 +01:00
@@ -74,7 +74,7 @@
        ipv6_addr_copy(&top_iph->daddr, (struct in6_addr *)&x->id.daddr);
 }
 
-static int xfrm6_tunnel_check_size(struct sk_buff *skb)
+static int xfrm6_tunnel_check_size(struct xfrm_state *x, struct sk_buff *skb)
 {
        int mtu, ret = 0;
        struct dst_entry *dst = skb->dst;
@@ -84,6 +84,7 @@
                mtu = IPV6_MIN_MTU;
 
        if (skb->len > mtu) {
+               spin_unlock_bh(&x->lock);
                icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu, skb->dev);
                ret = -EMSGSIZE;
        }
@@ -109,9 +110,10 @@
                goto error;
 
        if (x->props.mode) {
-               err = xfrm6_tunnel_check_size(skb);
+               err = xfrm6_tunnel_check_size(x, skb);
                if (err)
-                       goto error;
+                       /* xfrm6_tunnel_check_size drops the lock on error */
+                       goto error_nolock;
        }
 
        xfrm6_encap(skb);
<Prev in Thread] Current Thread [Next in Thread>