* More IPSEC trouble
@ 2005-03-10 12:45 Steve Hill
2005-03-10 15:01 ` Nicolas DICHTEL
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Steve Hill @ 2005-03-10 12:45 UTC (permalink / raw)
To: netdev
Unfortunately I've found another IPSEC kernel bug (2.6.10):
Running IPSEC in IPv4 tunnel mode - obviously the packets grow in size
when encrypted and when I send a packet that grows to > MTU size when
encrypted the machine locks up solid (the packet is generated by a
different machine that is routing it via the Linux IPSEC router whcih is
having problems). When it locks up I get no errors from the kernel - it
just freezes up solid.
All interfaces have an MTU of 1500 bytes. The IP address of the machine
generating the packet is 10.101.0.42 and the public address of it's ipsec
gateway (which is the one locking up) is "a.b.c.d" in the below logging.
The VPN server at the other end, also running the 2.6.10 kernel, is
w.x.y.z. The below logging shows a large ICMP packet that produces an
MTU-sized encrypted packet and works ok:
$ ping -n 10.254.3.19 -c 1 -s 1372
PING 10.254.3.19 (10.254.3.19) 1372(1400) bytes of data.
1380 bytes from 10.254.3.19: icmp_seq=0 ttl=62 time=94.8 ms
# tcpdump -n -i eth0 proto ICMP -v
12:35:50.064417 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto 1,
length: 1400) 10.101.0.42 > 10.254.3.19: icmp 1380: echo request seq 0
# tcpdump -n -i eth1 proto 50 or proto 51 or proto ICMP -v
12:28:53.427655 IP (tos 0x0, ttl 64, id 8224, offset 0, flags [DF], proto
51, length: 1500) a.b.c.d > w.x.y.z:
AH(spi=0x07260bb2,sumlen=16,seq=0x2d): IP (tos 0x0, ttl 64, id 55247,
offset 0, flags [DF], proto 50, length: 1456) a.b.c.d >
w.x.y.z: ESP(spi=0x041b1078,seq=0x2d)
Increasing the packet size by 1 byte would obviously require fragmentation
of the encrypted packet and causes the machine to lock up.
I will continue to do some debugging but any insight would be appreciated.
Thanks.
- Steve Hill (BSc)
Senior Software Developer Email: steve@navaho.co.uk
Navaho Technologies Ltd. Tel: +44-870-7034015
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-10 12:45 More IPSEC trouble Steve Hill
@ 2005-03-10 15:01 ` Nicolas DICHTEL
2005-03-10 16:32 ` Steve Hill
2005-03-10 17:15 ` Patrick McHardy
2005-03-11 1:17 ` Herbert Xu
2 siblings, 1 reply; 13+ messages in thread
From: Nicolas DICHTEL @ 2005-03-10 15:01 UTC (permalink / raw)
To: steve; +Cc: netdev
When the stack check the mtu for this packet, it doesn't know the size of
the overhead. So the total length of the packet don't include the size
of the esp or ah header. The same bug appears when you run
IPSEC in IPv4 transport mode over a 4in4 tunnel. A fix for this bug
is to allow locally the fragmentation of the packet.
Nicolas Dichtel
Here is a patch:
--- a/linux26/net/ipv4/xfrm4_output.c Thu Mar 10 15:50:30 2005
+++ b/linux26/net/ipv4/xfrm4_output.c Thu Mar 10 15:51:49 2005
@@ -116,6 +116,9 @@ int xfrm4_output(struct sk_buff *skb)
xfrm4_encap(skb);
+ /* We still allow to fragment this packet locally */
+ skb->local_df = 1;
+
err = x->type->output(skb);
if (err)
goto error;
Steve Hill wrote:
>
> Unfortunately I've found another IPSEC kernel bug (2.6.10):
>
> Running IPSEC in IPv4 tunnel mode - obviously the packets grow in size
> when encrypted and when I send a packet that grows to > MTU size when
> encrypted the machine locks up solid (the packet is generated by a
> different machine that is routing it via the Linux IPSEC router whcih
> is having problems). When it locks up I get no errors from the kernel
> - it just freezes up solid.
>
> All interfaces have an MTU of 1500 bytes. The IP address of the
> machine generating the packet is 10.101.0.42 and the public address of
> it's ipsec gateway (which is the one locking up) is "a.b.c.d" in the
> below logging. The VPN server at the other end, also running the
> 2.6.10 kernel, is w.x.y.z. The below logging shows a large ICMP
> packet that produces an MTU-sized encrypted packet and works ok:
>
> $ ping -n 10.254.3.19 -c 1 -s 1372
> PING 10.254.3.19 (10.254.3.19) 1372(1400) bytes of data.
> 1380 bytes from 10.254.3.19: icmp_seq=0 ttl=62 time=94.8 ms
>
> # tcpdump -n -i eth0 proto ICMP -v
> 12:35:50.064417 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF],
> proto 1, length: 1400) 10.101.0.42 > 10.254.3.19: icmp 1380: echo
> request seq 0
>
> # tcpdump -n -i eth1 proto 50 or proto 51 or proto ICMP -v
> 12:28:53.427655 IP (tos 0x0, ttl 64, id 8224, offset 0, flags [DF],
> proto 51, length: 1500) a.b.c.d > w.x.y.z:
> AH(spi=0x07260bb2,sumlen=16,seq=0x2d): IP (tos 0x0, ttl 64, id 55247,
> offset 0, flags [DF], proto 50, length: 1456) a.b.c.d > w.x.y.z:
> ESP(spi=0x041b1078,seq=0x2d)
>
> Increasing the packet size by 1 byte would obviously require
> fragmentation of the encrypted packet and causes the machine to lock up.
>
> I will continue to do some debugging but any insight would be
> appreciated. Thanks.
>
> - Steve Hill (BSc)
> Senior Software Developer Email:
> steve@navaho.co.uk
> Navaho Technologies Ltd. Tel: +44-870-7034015
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-10 15:01 ` Nicolas DICHTEL
@ 2005-03-10 16:32 ` Steve Hill
0 siblings, 0 replies; 13+ messages in thread
From: Steve Hill @ 2005-03-10 16:32 UTC (permalink / raw)
To: Nicolas DICHTEL; +Cc: netdev
On Thu, 10 Mar 2005, Nicolas DICHTEL wrote:
> When the stack check the mtu for this packet, it doesn't know the size of
> the overhead. So the total length of the packet don't include the size
> of the esp or ah header. The same bug appears when you run
> IPSEC in IPv4 transport mode over a 4in4 tunnel. A fix for this bug
> is to allow locally the fragmentation of the packet.
I've just tried the patch and unfortunately it doesn't fix the problem. :(
- Steve Hill (BSc)
Senior Software Developer Email: steve@navaho.co.uk
Navaho Technologies Ltd. Tel: +44-870-7034015
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-10 12:45 More IPSEC trouble Steve Hill
2005-03-10 15:01 ` Nicolas DICHTEL
@ 2005-03-10 17:15 ` Patrick McHardy
2005-03-11 1:17 ` Herbert Xu
2 siblings, 0 replies; 13+ messages in thread
From: Patrick McHardy @ 2005-03-10 17:15 UTC (permalink / raw)
To: Steve Hill; +Cc: netdev
Steve Hill wrote:
>
> Unfortunately I've found another IPSEC kernel bug (2.6.10):
>
> Increasing the packet size by 1 byte would obviously require
> fragmentation of the encrypted packet and causes the machine to lock up.
Doesn't sound familiar to me, but I would recommend trying
2.6.11 anyway.
Regards
Patrick
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-10 12:45 More IPSEC trouble Steve Hill
2005-03-10 15:01 ` Nicolas DICHTEL
2005-03-10 17:15 ` Patrick McHardy
@ 2005-03-11 1:17 ` Herbert Xu
2005-03-11 15:05 ` Steve Hill
2 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2005-03-11 1:17 UTC (permalink / raw)
To: Steve Hill; +Cc: netdev
Steve Hill <steve@services.navaho.net> wrote:
>
> Running IPSEC in IPv4 tunnel mode - obviously the packets grow in size
> when encrypted and when I send a packet that grows to > MTU size when
> encrypted the machine locks up solid (the packet is generated by a
> different machine that is routing it via the Linux IPSEC router whcih is
> having problems). When it locks up I get no errors from the kernel - it
> just freezes up solid.
What does CTRL-SCROLLLOCK or SysRq tell us?
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-11 1:17 ` Herbert Xu
@ 2005-03-11 15:05 ` Steve Hill
2005-03-12 0:00 ` Patrick McHardy
0 siblings, 1 reply; 13+ messages in thread
From: Steve Hill @ 2005-03-11 15:05 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
On Fri, 11 Mar 2005, Herbert Xu wrote:
> What does CTRL-SCROLLLOCK or SysRq tell us?
The kernel locks up solid - no SysRq, capslock/numlock lights don't toggle
when hitting the keys - completely dead.
I've managed to work out what causes it though:
I have overlapping subnets on the IPSEC tunnel - on the local side there
is 10.101.0.0/16 and on the remote side there is 10.0.0.0/8. The IPSEC
server is 10.101.0.254. The problem is that the policies I had required:
10.101.0.0/16 -> 10.0.0.0/8 Requires AH and ESP
10.0.0.0/8 -> 10.101.0.0/16 Requires AH and ESP
This obviously also matches traffic sent from 10.101.0.254 (the IPSEC
server) to any machine on the local 10.101.0.0/16 network. And since
there is no SA for that traffic it gets dropped.
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.
So whilest the error in the configuration legitimately causes parts of the
network to not work, it certainly shouldn't have caused the kernel to lock
up. It seems the problem is occurring when the kernel generates a packet
which the policy drops.
I have fixed my configuration now so that I have policies like:
10.101.0.0/16 -> 10.101.0.0/16 None
10.101.0.0/16 -> 10.0.0.0/8 Requires AH and ESP
10.0.0.0/8 -> 10.101.0.0/16 Requires AH and ESP
Since the policy nolonger catches the kernel-generated packets the problem
nolonger occurs for me, but obviously there is a bug there that should
really be fixed.
- Steve Hill (BSc)
Senior Software Developer Email: steve@navaho.co.uk
Navaho Technologies Ltd. Tel: +44-870-7034015
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-11 15:05 ` Steve Hill
@ 2005-03-12 0:00 ` Patrick McHardy
2005-03-12 0:11 ` Patrick McHardy
2005-03-12 1:35 ` Herbert Xu
0 siblings, 2 replies; 13+ messages in thread
From: Patrick McHardy @ 2005-03-12 0:00 UTC (permalink / raw)
To: Steve Hill; +Cc: Herbert Xu, netdev, David S. Miller
[-- Attachment #1: Type: text/plain, Size: 568 bytes --]
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.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1845 bytes --]
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2005/03/12 00:59:07+01:00 kaber@coreworks.de
# [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
#
# net/ipv6/xfrm6_output.c
# 2005/03/12 00:58:59+01:00 kaber@coreworks.de +3 -1
# [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
#
# net/ipv4/xfrm4_output.c
# 2005/03/12 00:58:59+01:00 kaber@coreworks.de +3 -1
# [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
#
diff -Nru a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
--- a/net/ipv4/xfrm4_output.c 2005-03-12 00:59:22 +01:00
+++ b/net/ipv4/xfrm4_output.c 2005-03-12 00:59:22 +01:00
@@ -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;
}
@@ -111,7 +112,8 @@
if (x->props.mode) {
err = xfrm4_tunnel_check_size(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 00:59:22 +01:00
+++ b/net/ipv6/xfrm6_output.c 2005-03-12 00:59:22 +01:00
@@ -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;
}
@@ -111,7 +112,8 @@
if (x->props.mode) {
err = xfrm6_tunnel_check_size(skb);
if (err)
- goto error;
+ /* xfrm6_tunnel_check_size() drops the lock on error */
+ goto error_nolock;
}
xfrm6_encap(skb);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-12 0:00 ` Patrick McHardy
@ 2005-03-12 0:11 ` Patrick McHardy
2005-03-12 0:13 ` Patrick McHardy
2005-03-12 1:35 ` Herbert Xu
1 sibling, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2005-03-12 0:11 UTC (permalink / raw)
To: Steve Hill; +Cc: Herbert Xu, netdev, David S. Miller
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
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.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1843 bytes --]
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2005/03/12 00:59:07+01:00 kaber@coreworks.de
# [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
#
# net/ipv6/xfrm6_output.c
# 2005/03/12 00:58:59+01:00 kaber@coreworks.de +3 -1
# [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
#
# net/ipv4/xfrm4_output.c
# 2005/03/12 00:58:59+01:00 kaber@coreworks.de +3 -1
# [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
#
diff -Nru a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
--- a/net/ipv4/xfrm4_output.c 2005-03-12 01:03:18 +01:00
+++ b/net/ipv4/xfrm4_output.c 2005-03-12 01:03:18 +01:00
@@ -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;
}
@@ -111,7 +112,8 @@
if (x->props.mode) {
err = xfrm4_tunnel_check_size(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:03:18 +01:00
+++ b/net/ipv6/xfrm6_output.c 2005-03-12 01:03:18 +01:00
@@ -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;
}
@@ -111,7 +112,8 @@
if (x->props.mode) {
err = xfrm6_tunnel_check_size(skb);
if (err)
- goto error;
+ /* xfrm6_tunnel_check_size drops the lock on error */
+ goto error_nolock;
}
xfrm6_encap(skb);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-12 0:11 ` Patrick McHardy
@ 2005-03-12 0:13 ` Patrick McHardy
0 siblings, 0 replies; 13+ messages in thread
From: Patrick McHardy @ 2005-03-12 0:13 UTC (permalink / raw)
To: Steve Hill; +Cc: Herbert Xu, netdev, David S. Miller
[-- Attachment #1: Type: text/plain, Size: 808 bytes --]
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 ..
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2515 bytes --]
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2005/03/12 01:10:40+01:00 kaber@coreworks.de
# [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
#
# net/ipv6/xfrm6_output.c
# 2005/03/12 01:10:31+01:00 kaber@coreworks.de +5 -3
# [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
#
# net/ipv4/xfrm4_output.c
# 2005/03/12 01:10:31+01:00 kaber@coreworks.de +5 -3
# [XFRM]: Avoid possible deadlock for locally generated ICMP errors
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
#
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);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-12 0:00 ` Patrick McHardy
2005-03-12 0:11 ` Patrick McHardy
@ 2005-03-12 1:35 ` Herbert Xu
2005-03-12 2:35 ` Patrick McHardy
2005-03-14 12:12 ` Steve Hill
1 sibling, 2 replies; 13+ messages in thread
From: Herbert Xu @ 2005-03-12 1:35 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Steve Hill, netdev, David S. Miller
[-- Attachment #1: Type: text/plain, Size: 757 bytes --]
On Sat, Mar 12, 2005 at 01:00:37AM +0100, Patrick McHardy wrote:
>
> 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.
Good catch. However let's try to avoid spreading lock/unlock calls
around if possible.
Since tunnel_check_size only looks at the state to determine whether
it's tunnel mode, it doesn't need to hold the lock at all.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 1078 bytes --]
===== net/ipv4/xfrm4_output.c 1.7 vs edited =====
--- 1.7/net/ipv4/xfrm4_output.c 2005-03-07 16:33:59 +11:00
+++ edited/net/ipv4/xfrm4_output.c 2005-03-12 12:26:20 +11:00
@@ -103,16 +103,16 @@
goto error_nolock;
}
- spin_lock_bh(&x->lock);
- err = xfrm_state_check(x, skb);
- if (err)
- goto error;
-
if (x->props.mode) {
err = xfrm4_tunnel_check_size(skb);
if (err)
- goto error;
+ goto error_nolock;
}
+
+ spin_lock_bh(&x->lock);
+ err = xfrm_state_check(x, skb);
+ if (err)
+ goto error;
xfrm4_encap(skb);
===== net/ipv6/xfrm6_output.c 1.9 vs edited =====
--- 1.9/net/ipv6/xfrm6_output.c 2005-03-07 16:33:59 +11:00
+++ edited/net/ipv6/xfrm6_output.c 2005-03-12 12:29:00 +11:00
@@ -103,16 +103,16 @@
goto error_nolock;
}
- spin_lock_bh(&x->lock);
- err = xfrm_state_check(x, skb);
- if (err)
- goto error;
-
if (x->props.mode) {
err = xfrm6_tunnel_check_size(skb);
if (err)
- goto error;
+ goto error_nolock;
}
+
+ spin_lock_bh(&x->lock);
+ err = xfrm_state_check(x, skb);
+ if (err)
+ goto error;
xfrm6_encap(skb);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-12 1:35 ` Herbert Xu
@ 2005-03-12 2:35 ` Patrick McHardy
2005-03-14 12:12 ` Steve Hill
1 sibling, 0 replies; 13+ messages in thread
From: Patrick McHardy @ 2005-03-12 2:35 UTC (permalink / raw)
To: Herbert Xu; +Cc: Steve Hill, netdev, David S. Miller
Herbert Xu wrote:
> Good catch. However let's try to avoid spreading lock/unlock calls
> around if possible.
>
> Since tunnel_check_size only looks at the state to determine whether
> it's tunnel mode, it doesn't need to hold the lock at all.
Agreed, your patch is better.
Regards
Patrick
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-12 1:35 ` Herbert Xu
2005-03-12 2:35 ` Patrick McHardy
@ 2005-03-14 12:12 ` Steve Hill
2005-03-15 5:39 ` David S. Miller
1 sibling, 1 reply; 13+ messages in thread
From: Steve Hill @ 2005-03-14 12:12 UTC (permalink / raw)
To: Herbert Xu; +Cc: Patrick McHardy, netdev, David S. Miller
On Sat, 12 Mar 2005, Herbert Xu wrote:
> Since tunnel_check_size only looks at the state to determine whether
> it's tunnel mode, it doesn't need to hold the lock at all.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
I can confirm this fixes the problem - tested under 2.6.10.
- Steve Hill (BSc)
Senior Software Developer Email: steve@navaho.co.uk
Navaho Technologies Ltd. Tel: +44-870-7034015
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: More IPSEC trouble
2005-03-14 12:12 ` Steve Hill
@ 2005-03-15 5:39 ` David S. Miller
0 siblings, 0 replies; 13+ messages in thread
From: David S. Miller @ 2005-03-15 5:39 UTC (permalink / raw)
To: Steve Hill; +Cc: herbert, kaber, netdev
On Mon, 14 Mar 2005 12:12:27 +0000 (GMT)
Steve Hill <steve@services.navaho.net> wrote:
> On Sat, 12 Mar 2005, Herbert Xu wrote:
>
> > Since tunnel_check_size only looks at the state to determine whether
> > it's tunnel mode, it doesn't need to hold the lock at all.
> >
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> I can confirm this fixes the problem - tested under 2.6.10.
I've applied Herbert's patch, thanks guys.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2005-03-15 5:39 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-03-10 12:45 More IPSEC trouble Steve Hill
2005-03-10 15:01 ` Nicolas DICHTEL
2005-03-10 16:32 ` Steve Hill
2005-03-10 17:15 ` Patrick McHardy
2005-03-11 1:17 ` Herbert Xu
2005-03-11 15:05 ` Steve Hill
2005-03-12 0:00 ` Patrick McHardy
2005-03-12 0:11 ` Patrick McHardy
2005-03-12 0:13 ` Patrick McHardy
2005-03-12 1:35 ` Herbert Xu
2005-03-12 2:35 ` Patrick McHardy
2005-03-14 12:12 ` Steve Hill
2005-03-15 5:39 ` David S. Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).