netdev - Re: [PATCH] tcp FRTO: in-order-only "TCP proxy" fragility workaround

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 20 Aug 2008 15:37:13 +0300 (EEST)
From:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>
To:	"Dâniel Fraga" <fragabr@...il.com>
cc:	David Miller <davem@...emloft.net>, thomas.jarosch@...ra2net.com,
	billfink@...dspring.com, Netdev <netdev@...r.kernel.org>,
	Patrick Hardy <kaber@...sh.net>,
	netfilter-devel@...r.kernel.org, kadlec@...ckhole.kfki.hu
Subject: Re: [PATCH] tcp FRTO: in-order-only "TCP proxy" fragility workaround

On Tue, 19 Aug 2008, Dâniel Fraga wrote:

> On Tue, 19 Aug 2008 13:38:35 +0300 (EEST)
> "Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi> wrote:
> 
> > Perhaps, though it's not at all clear how it could do that...
> 
> 	I was thinking here of of some specific configuration I use.
> For example, I always used the wonder shaper htb script:
> 
> http://lartc.org/howto/lartc.cookbook.ultimate-tc.html#AEN2241
> 
> 	Could HTB mess with frto or cause this problem? Would it be
> useful to disable completely HTB and use just the default scheduler?

Based on irc discussion with davem, there is a htb bug which can cause 
corruption of the retransmitted TCP packets (and then a discard due to 
checksum mismatch). That would also explain the strange headers I noticed 
earlier. There's a patch below (should apply to 2.6.26), please put it at 
least on the host(s) which use htb (I don't know if both server and the 
client do use wondershaper script or just the client). An different 
failure symptoms (one could be somehow frto related as FRTO is used while 
retransmitting) are also quite well explainable.

But FRTO is mostly not a suspect based on the tcpdump you provided (no 
FRTO workaround would help in that).

If you tcpdump with -s0 at receiver, you get full payload and therefore 
it is possible to verify checksum correctness.

> > Do you have net namespaces enabled CONFIG_NET_NS in .config?
> 
> 	I couldn't find this specific option:
> 
> fraga@tux /usr/src/linux$ grep CONFIG_NET_NS .config
> fraga@tux /usr/src/linux$ 
> 
> 	But I have those:

I wasn't in error :-), it took some time also for me to figure out the 
right one, it's quite expected to be off (and even that missing) since 
it's currently !SYSFS depending.

> > Any netfilter (iptables) rules on server which could cause those packets 
> > to not reach TCP layer?
> 
> 	Here are the complete rules:

...snip...

> 	As you can see, it's a preetty simple set of rules, nothing exotic here.

...agreed.

-- 
 i.


---
From: David Miller <davem@...emloft.net>

pkt_sched: Fix return value corruption in HTB and TBF.

Packet schedulers should only return NET_XMIT_DROP iff
the packet really was dropped.  If the packet does reach
the device after we return NET_XMIT_DROP then TCP can
crash because it depends upon the enqueue path return
values being accurate.

Signed-off-by: David S. Miller <davem@...emloft.net>

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 3fb58f4..51c3f68 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -595,11 +595,13 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 		kfree_skb(skb);
 		return ret;
 #endif
-	} else if (cl->un.leaf.q->enqueue(skb, cl->un.leaf.q) !=
+	} else if ((ret = cl->un.leaf.q->enqueue(skb, cl->un.leaf.q)) !=
 		   NET_XMIT_SUCCESS) {
-		sch->qstats.drops++;
-		cl->qstats.drops++;
-		return NET_XMIT_DROP;
+		if (ret == NET_XMIT_DROP) {
+			sch->qstats.drops++;
+			cl->qstats.drops++;
+		}
+		return ret;
 	} else {
 		cl->bstats.packets +=
 			skb_is_gso(skb)?skb_shinfo(skb)->gso_segs:1;
@@ -639,11 +641,13 @@ static int htb_requeue(struct sk_buff *skb, struct Qdisc *sch)
 		kfree_skb(skb);
 		return ret;
 #endif
-	} else if (cl->un.leaf.q->ops->requeue(skb, cl->un.leaf.q) !=
+	} else if ((ret = cl->un.leaf.q->ops->requeue(skb, cl->un.leaf.q)) !=
 		   NET_XMIT_SUCCESS) {
-		sch->qstats.drops++;
-		cl->qstats.drops++;
-		return NET_XMIT_DROP;
+		if (ret == NET_XMIT_DROP) {
+			sch->qstats.drops++;
+			cl->qstats.drops++;
+		}
+		return ret;
 	} else
 		htb_activate(q, cl);
 
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 0b7d78f..fc6f8f3 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -123,15 +123,8 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 	struct tbf_sched_data *q = qdisc_priv(sch);
 	int ret;
 
-	if (skb->len > q->max_size) {
-		sch->qstats.drops++;
-#ifdef CONFIG_NET_CLS_ACT
-		if (sch->reshape_fail == NULL || sch->reshape_fail(skb, sch))
-#endif
-			kfree_skb(skb);
-
-		return NET_XMIT_DROP;
-	}
+	if (skb->len > q->max_size)
+		return qdisc_reshape_fail(skb, sch);
 
 	if ((ret = q->qdisc->enqueue(skb, q->qdisc)) != 0) {
 		sch->qstats.drops++;