[05/08] [IPSEC]: Do not hold state lock while checking size

[05/08] [IPSEC]: Do not hold state lock while checking size

[Greg KH]

-stable review patch.  If anyone has any objections, please let us know.

------------------

This patch from Herbert Xu fixes a deadlock with IPsec.
When an ICMP frag. required is sent and the ICMP message
needs the same SA as the packet that caused it the state
will be locked twice.

[IPSEC]: Do not hold state lock while checking size.

This can elicit ICMP message output and thus result in a
deadlock.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Chris Wright <chrisw@osdl.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff -Nru a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
--- a/net/ipv4/xfrm4_output.c	2005-03-20 16:53:05 +01:00
+++ b/net/ipv4/xfrm4_output.c	2005-03-20 16:53:05 +01: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);
 
diff -Nru a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
--- a/net/ipv6/xfrm6_output.c	2005-03-20 16:53:05 +01:00
+++ b/net/ipv6/xfrm6_output.c	2005-03-20 16:53:05 +01: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);
 

[07/08] [TCP] Fix BIC congestion avoidance algorithm error

[Greg KH]

-stable review patch.  If anyone has any objections, please let us know.

------------------

Since BIC is the default congestion control algorithm
enabled in every 2.6.x kernel out there, fixing errors
in it becomes quite critical.

A flaw in the loss handling caused it to not perform
the binary search regimen of the BIC algorithm
properly.

The fix below from Stephen Hemminger has been heavily
verified.

[TCP]: BIC not binary searching correctly

While redoing BIC for the split up version, I discovered that the existing
2.6.11 code doesn't really do binary search. It ends up being just a slightly
modified version of Reno.  See attached graphs to see the effect over simulated
1mbit environment.

The problem is that BIC is supposed to reset the cwnd to the last loss value
rather than ssthresh when loss is detected.  The correct code (from the BIC
TCP code for Web100) is in this patch.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Chris Wright <chrisw@osdl.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

--- 1.92/net/ipv4/tcp_input.c	2005-02-22 10:45:31 -08:00
+++ edited/net/ipv4/tcp_input.c	2005-03-23 10:55:18 -08:00
@@ -1653,7 +1653,10 @@
 static void tcp_undo_cwr(struct tcp_sock *tp, int undo)
 {
 	if (tp->prior_ssthresh) {
-		tp->snd_cwnd = max(tp->snd_cwnd, tp->snd_ssthresh<<1);
+		if (tcp_is_bic(tp))
+			tp->snd_cwnd = max(tp->snd_cwnd, tp->bictcp.last_max_cwnd);
+		else
+			tp->snd_cwnd = max(tp->snd_cwnd, tp->snd_ssthresh<<1);
 
 		if (undo && tp->prior_ssthresh > tp->snd_ssthresh) {
 			tp->snd_ssthresh = tp->prior_ssthresh;

Re: [07/08] [TCP] Fix BIC congestion avoidance algorithm error

[Theodore Ts'o]

On Tue, Apr 05, 2005 at 09:47:59AM -0700, Greg KH wrote:
> 
> -stable review patch.  If anyone has any objections, please let us know.
> 
> While redoing BIC for the split up version, I discovered that the
> existing 2.6.11 code doesn't really do binary search. It ends up
> being just a slightly modified version of Reno.  See attached graphs
> to see the effect over simulated 1mbit environment.

I hate to be a stickler for the rules, but does this really meet this
criteria?

 - It must fix a real bug that bothers people (not a, "This could be a
   problem..." type thing.)

If the congestion control alogirthm is "Reno-like", what is
user-visible impact to users?  There are OS's out there with TCP/IP
stacks that are still using Reno, aren't there?  

Knowing the answer to the question, "How does this bug `bother' either
users or network administrators?" would probably be really helpful.

						- Ted

Re: [07/08] [TCP] Fix BIC congestion avoidance algorithm error

[David S. Miller]

On Tue, 5 Apr 2005 14:22:02 -0400
Theodore Ts'o <tytso@mit.edu> wrote:

> If the congestion control alogirthm is "Reno-like", what is
> user-visible impact to users?  There are OS's out there with TCP/IP
> stacks that are still using Reno, aren't there?  

An incorrect implementation of any congestion control algorithm
has ramifications not considered when the congestion control
author verified the design of his algorithm.

This has a large impact on every user on the internet, not just
Linux machines.

Perhaps on a microscopic scale "this" part of the BIC algorithm
was just behaving Reno-like due to the bug, but what implications
does that error have as applied to the other heuristics in BIC?
This is what I'm talking about.  BIC operates in several modes,
one of which is a pseudo binary search mode, and another is a
less aggressive slower increase mode.

Therefore I think fixes to congestion control algorithms which
are enabled by default always should take a high priority in
the stable kernels.

Re: [07/08] [TCP] Fix BIC congestion avoidance algorithm error

[Stephen Hemminger]

On Tue, 5 Apr 2005 11:26:08 -0700
"David S. Miller" <davem@davemloft.net> wrote:

> On Tue, 5 Apr 2005 14:22:02 -0400
> Theodore Ts'o <tytso@mit.edu> wrote:
> 
> > If the congestion control alogirthm is "Reno-like", what is
> > user-visible impact to users?  There are OS's out there with TCP/IP
> > stacks that are still using Reno, aren't there?  
> 
> An incorrect implementation of any congestion control algorithm
> has ramifications not considered when the congestion control
> author verified the design of his algorithm.
> 
> This has a large impact on every user on the internet, not just
> Linux machines.
> 
> Perhaps on a microscopic scale "this" part of the BIC algorithm
> was just behaving Reno-like due to the bug, but what implications
> does that error have as applied to the other heuristics in BIC?
> This is what I'm talking about.  BIC operates in several modes,
> one of which is a pseudo binary search mode, and another is a
> less aggressive slower increase mode.

> Therefore I think fixes to congestion control algorithms which
> are enabled by default always should take a high priority in
> the stable kernels.

Also, hopefully distro vendors will pick up 2.6.11.X fixes and update their customers.