linux-kernel - Re: [PATCH net-next-2.6] net/ipv4: push IP options to CB in ip

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]

Message-ID: <1283232031.2405.38.camel@edumazet-laptop>
Date:	Tue, 31 Aug 2010 07:20:31 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Bandan Das <bandan.das@...atus.com>,
	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	David Miller <davem@...emloft.net>,
	NetDev <netdev@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Patrick McHardy <kaber@...sh.net>
Subject: Re: [PATCH net-next-2.6] net/ipv4: push IP options to CB in
 ip_fragment

Le lundi 30 août 2010 à 19:21 -0400, Bandan Das a écrit :
> > 
> > I wonder if we want this.
> > 
> > Maybe setting skb->local_df = 1 in bridge should be enough ?
> > 
> > 
> Thanks Eric for looking at this. Indeed, setting local_df to 1 seems to be enough! I will
> respin and post a different patch.
> 

Reading this stuff again, I wonder if we should not revert commit
17762060c25590bfddd  and use a different trick

Author: Herbert Xu <herbert@...dor.apana.org.au>
Date:   Mon Jul 5 21:29:28 2010 +0000

    bridge: Clear IPCB before possible entry into IP stack
    
    The bridge protocol lives dangerously by having incestuous relations
    with the IP stack.  In this instance an abomination has been created
    where a bogus IPCB area from a bridged packet leads to a crash in
    the IP stack because it's interpreted as IP options.
    
    This patch papers over the problem by clearing the IPCB area in that
    particular spot.  To fix this properly we'd also need to parse any
    IP options if present but I'm way too lazy for that.
    
    Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
    Signed-off-by: David S. Miller <davem@...emloft.net>


We could have a padding in front of struct br_input_skb_cb to make sure
we dont overwrite IP (4|6) CB in bridge ?

Something like this untested patch :

diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 2c911c0..9fdf1b1 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -598,9 +598,6 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb,
 
 	pskb_trim_rcsum(skb, len);
 
-	/* BUG: Should really parse the IP options here. */
-	memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
-
 	nf_bridge_put(skb->nf_bridge);
 	if (!nf_bridge_alloc(skb))
 		return NF_DROP;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 75c90ed..b27163a 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -18,6 +18,7 @@
 #include <linux/netpoll.h>
 #include <linux/u64_stats_sync.h>
 #include <net/route.h>
+#include <linux/ipv6.h>
 
 #define BR_HASH_BITS 8
 #define BR_HASH_SIZE (1 << BR_HASH_BITS)
@@ -244,6 +245,8 @@ struct net_bridge
 };
 
 struct br_input_skb_cb {
+	struct inet6_skb_parm pad;
+
 	struct net_device *brdev;
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	int igmp;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/