* [PATCH 1/4] bridge-netfilter: memory corruption fix
2006-08-23 0:10 [PATCH 0/4] bridge-netfilter fixes shemminger
@ 2006-08-23 0:10 ` shemminger
2006-08-23 0:10 ` [PATCH 2/4] bridge-netfilter: code rearrangement for clarity shemminger
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: shemminger @ 2006-08-23 0:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev
[-- Attachment #1: br-nf-over.patch --]
[-- Type: text/plain, Size: 1587 bytes --]
The bridge-netfilter code will overwrite memory if there is not headroom
in the skb to save the header. This first showed up when using Xen with
sky2 driver that doesn't allocate the extra space.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- br-nf.orig/include/linux/netfilter_bridge.h 2006-08-22 16:43:41.000000000 -0700
+++ br-nf/include/linux/netfilter_bridge.h 2006-08-22 16:45:05.000000000 -0700
@@ -48,15 +48,25 @@
/* Only used in br_forward.c */
static inline
-void nf_bridge_maybe_copy_header(struct sk_buff *skb)
+int nf_bridge_maybe_copy_header(struct sk_buff *skb)
{
+ int err;
+
if (skb->nf_bridge) {
if (skb->protocol == __constant_htons(ETH_P_8021Q)) {
+ err = skb_cow(skb, 18);
+ if (err)
+ return err;
memcpy(skb->data - 18, skb->nf_bridge->data, 18);
skb_push(skb, 4);
- } else
+ } else {
+ err = skb_cow(skb, 16);
+ if (err)
+ return err;
memcpy(skb->data - 16, skb->nf_bridge->data, 16);
+ }
}
+ return 0;
}
/* This is called by the IP fragmenting code and it ensures there is
--- br-nf.orig/net/bridge/br_forward.c 2006-08-22 16:43:41.000000000 -0700
+++ br-nf/net/bridge/br_forward.c 2006-08-22 16:44:04.000000000 -0700
@@ -40,11 +40,15 @@
else {
#ifdef CONFIG_BRIDGE_NETFILTER
/* ip_refrag calls ip_fragment, doesn't copy the MAC header. */
- nf_bridge_maybe_copy_header(skb);
+ if (nf_bridge_maybe_copy_header(skb))
+ kfree_skb(skb);
+ else
#endif
- skb_push(skb, ETH_HLEN);
+ {
+ skb_push(skb, ETH_HLEN);
- dev_queue_xmit(skb);
+ dev_queue_xmit(skb);
+ }
}
return 0;
--
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/4] bridge-netfilter: code rearrangement for clarity
2006-08-23 0:10 [PATCH 0/4] bridge-netfilter fixes shemminger
2006-08-23 0:10 ` [PATCH 1/4] bridge-netfilter: memory corruption fix shemminger
@ 2006-08-23 0:10 ` shemminger
2006-08-23 0:10 ` [PATCH 3/4] bridge-netfilter: simplify nf_bridge_pad shemminger
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: shemminger @ 2006-08-23 0:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev
[-- Attachment #1: br-nf-reorg.patch --]
[-- Type: text/plain, Size: 3085 bytes --]
Cleanup and rearrangement for better style and clarity:
Split the function nf_bridge_maybe_copy_header into two pieces
Move copy portion out of line.
Use Ethernet header size macros.
Use header file to handle CONFIG_NETFILTER_BRIDGE differences
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- br-nf.orig/include/linux/netfilter_bridge.h 2006-08-22 16:45:05.000000000 -0700
+++ br-nf/include/linux/netfilter_bridge.h 2006-08-22 16:46:47.000000000 -0700
@@ -47,26 +47,12 @@
/* Only used in br_forward.c */
-static inline
-int nf_bridge_maybe_copy_header(struct sk_buff *skb)
+extern int nf_bridge_copy_header(struct sk_buff *skb);
+static inline int nf_bridge_maybe_copy_header(struct sk_buff *skb)
{
- int err;
-
- if (skb->nf_bridge) {
- if (skb->protocol == __constant_htons(ETH_P_8021Q)) {
- err = skb_cow(skb, 18);
- if (err)
- return err;
- memcpy(skb->data - 18, skb->nf_bridge->data, 18);
- skb_push(skb, 4);
- } else {
- err = skb_cow(skb, 16);
- if (err)
- return err;
- memcpy(skb->data - 16, skb->nf_bridge->data, 16);
- }
- }
- return 0;
+ if (skb->nf_bridge)
+ return nf_bridge_copy_header(skb);
+ return 0;
}
/* This is called by the IP fragmenting code and it ensures there is
@@ -90,6 +76,8 @@
};
extern int brnf_deferred_hooks;
+#else
+#define nf_bridge_maybe_copy_header(skb) (0)
#endif /* CONFIG_BRIDGE_NETFILTER */
#endif /* __KERNEL__ */
--- br-nf.orig/net/bridge/br_forward.c 2006-08-22 16:44:04.000000000 -0700
+++ br-nf/net/bridge/br_forward.c 2006-08-22 16:48:12.000000000 -0700
@@ -38,13 +38,10 @@
if (packet_length(skb) > skb->dev->mtu && !skb_is_gso(skb))
kfree_skb(skb);
else {
-#ifdef CONFIG_BRIDGE_NETFILTER
/* ip_refrag calls ip_fragment, doesn't copy the MAC header. */
if (nf_bridge_maybe_copy_header(skb))
kfree_skb(skb);
- else
-#endif
- {
+ else {
skb_push(skb, ETH_HLEN);
dev_queue_xmit(skb);
--- br-nf.orig/net/bridge/br_netfilter.c 2006-08-22 16:44:04.000000000 -0700
+++ br-nf/net/bridge/br_netfilter.c 2006-08-22 16:48:56.000000000 -0700
@@ -127,14 +127,37 @@
static inline void nf_bridge_save_header(struct sk_buff *skb)
{
- int header_size = 16;
+ int header_size = ETH_HLEN;
if (skb->protocol == htons(ETH_P_8021Q))
- header_size = 18;
+ header_size += VLAN_HLEN;
memcpy(skb->nf_bridge->data, skb->data - header_size, header_size);
}
+/*
+ * When forwarding bridge frames, we save a copy of the original
+ * header before processing.
+ */
+int nf_bridge_copy_header(struct sk_buff *skb)
+{
+ int err;
+ int header_size = ETH_HLEN;
+
+ if (skb->protocol == htons(ETH_P_8021Q))
+ header_size += VLAN_HLEN;
+
+ err = skb_cow(skb, header_size);
+ if (err)
+ return err;
+
+ memcpy(skb->data - header_size, skb->nf_bridge->data, header_size);
+
+ if (skb->protocol == htons(ETH_P_8021Q))
+ __skb_push(skb, VLAN_HLEN);
+ return 0;
+}
+
/* PF_BRIDGE/PRE_ROUTING *********************************************/
/* Undo the changes made for ip6tables PREROUTING and continue the
* bridge PRE_ROUTING hook. */
--
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/4] bridge-netfilter: simplify nf_bridge_pad
2006-08-23 0:10 [PATCH 0/4] bridge-netfilter fixes shemminger
2006-08-23 0:10 ` [PATCH 1/4] bridge-netfilter: memory corruption fix shemminger
2006-08-23 0:10 ` [PATCH 2/4] bridge-netfilter: code rearrangement for clarity shemminger
@ 2006-08-23 0:10 ` shemminger
2006-08-23 0:10 ` [PATCH 4/4] bridge-netfilter: debug message fixes shemminger
2006-08-27 3:27 ` [PATCH 0/4] bridge-netfilter fixes David Miller
4 siblings, 0 replies; 6+ messages in thread
From: shemminger @ 2006-08-23 0:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev
[-- Attachment #1: br-nf-pad.patch --]
[-- Type: text/plain, Size: 2441 bytes --]
Do some simple optimization on the nf_bridge_pad() function
and don't use magic constants. Eliminate a double call and
the #ifdef'd code for CONFIG_BRIDGE_NETFILTER.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- br-nf.orig/include/linux/netfilter_bridge.h 2006-08-22 16:46:47.000000000 -0700
+++ br-nf/include/linux/netfilter_bridge.h 2006-08-22 16:52:25.000000000 -0700
@@ -5,9 +5,8 @@
*/
#include <linux/netfilter.h>
-#if defined(__KERNEL__) && defined(CONFIG_BRIDGE_NETFILTER)
#include <linux/if_ether.h>
-#endif
+#include <linux/if_vlan.h>
/* Bridge Hooks */
/* After promisc drops, checksum checks. */
@@ -57,16 +56,10 @@
/* This is called by the IP fragmenting code and it ensures there is
* enough room for the encapsulating header (if there is one). */
-static inline
-int nf_bridge_pad(struct sk_buff *skb)
+static inline int nf_bridge_pad(const struct sk_buff *skb)
{
- if (skb->protocol == __constant_htons(ETH_P_IP))
- return 0;
- if (skb->nf_bridge) {
- if (skb->protocol == __constant_htons(ETH_P_8021Q))
- return 4;
- }
- return 0;
+ return (skb->nf_bridge && skb->protocol == htons(ETH_P_8021Q))
+ ? VLAN_HLEN : 0;
}
struct bridge_skb_cb {
@@ -78,6 +71,7 @@
extern int brnf_deferred_hooks;
#else
#define nf_bridge_maybe_copy_header(skb) (0)
+#define nf_bridge_pad(skb) (0)
#endif /* CONFIG_BRIDGE_NETFILTER */
#endif /* __KERNEL__ */
--- br-nf.orig/net/ipv4/ip_output.c 2006-08-22 16:43:41.000000000 -0700
+++ br-nf/net/ipv4/ip_output.c 2006-08-22 16:51:22.000000000 -0700
@@ -425,7 +425,7 @@
int ptr;
struct net_device *dev;
struct sk_buff *skb2;
- unsigned int mtu, hlen, left, len, ll_rs;
+ unsigned int mtu, hlen, left, len, ll_rs, pad;
int offset;
__be16 not_last_frag;
struct rtable *rt = (struct rtable*)skb->dst;
@@ -554,14 +554,13 @@
left = skb->len - hlen; /* Space per frame */
ptr = raw + hlen; /* Where to start from */
-#ifdef CONFIG_BRIDGE_NETFILTER
/* for bridged IP traffic encapsulated inside f.e. a vlan header,
- * we need to make room for the encapsulating header */
- ll_rs = LL_RESERVED_SPACE_EXTRA(rt->u.dst.dev, nf_bridge_pad(skb));
- mtu -= nf_bridge_pad(skb);
-#else
- ll_rs = LL_RESERVED_SPACE(rt->u.dst.dev);
-#endif
+ * we need to make room for the encapsulating header
+ */
+ pad = nf_bridge_pad(skb);
+ ll_rs = LL_RESERVED_SPACE_EXTRA(rt->u.dst.dev, pad);
+ mtu -= pad;
+
/*
* Fragment the datagram.
*/
--
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 4/4] bridge-netfilter: debug message fixes
2006-08-23 0:10 [PATCH 0/4] bridge-netfilter fixes shemminger
` (2 preceding siblings ...)
2006-08-23 0:10 ` [PATCH 3/4] bridge-netfilter: simplify nf_bridge_pad shemminger
@ 2006-08-23 0:10 ` shemminger
2006-08-27 3:27 ` [PATCH 0/4] bridge-netfilter fixes David Miller
4 siblings, 0 replies; 6+ messages in thread
From: shemminger @ 2006-08-23 0:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev
[-- Attachment #1: br-nf-msg.patch --]
[-- Type: text/plain, Size: 1610 bytes --]
If CONFIG_NETFILTER_DEBUG is enabled, it shouldn't change the
actions of the filtering. The message about skb->dst being NULL
is commonly triggered by dhclient, so it is useless. Make sure all
messages end in newline.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- br-nf.orig/net/bridge/br_netfilter.c 2006-08-22 16:48:56.000000000 -0700
+++ br-nf/net/bridge/br_netfilter.c 2006-08-22 17:07:28.000000000 -0700
@@ -718,16 +718,6 @@
else
pf = PF_INET6;
-#ifdef CONFIG_NETFILTER_DEBUG
- /* Sometimes we get packets with NULL ->dst here (for example,
- * running a dhcp client daemon triggers this). This should now
- * be fixed, but let's keep the check around. */
- if (skb->dst == NULL) {
- printk(KERN_CRIT "br_netfilter: skb->dst == NULL.");
- return NF_ACCEPT;
- }
-#endif
-
nf_bridge = skb->nf_bridge;
nf_bridge->physoutdev = skb->dev;
realindev = nf_bridge->physindev;
@@ -809,7 +799,7 @@
* keep the check just to be sure... */
if (skb->mac.raw < skb->head || skb->mac.raw + ETH_HLEN > skb->data) {
printk(KERN_CRIT "br_netfilter: Argh!! br_nf_post_routing: "
- "bad mac.raw pointer.");
+ "bad mac.raw pointer.\n");
goto print_error;
}
#endif
@@ -827,7 +817,7 @@
#ifdef CONFIG_NETFILTER_DEBUG
if (skb->dst == NULL) {
- printk(KERN_CRIT "br_netfilter: skb->dst == NULL.");
+ printk(KERN_INFO "br_netfilter post_routing: skb->dst == NULL\n");
goto print_error;
}
#endif
@@ -864,6 +854,7 @@
}
printk(" head:%p, raw:%p, data:%p\n", skb->head, skb->mac.raw,
skb->data);
+ dump_stack();
return NF_ACCEPT;
#endif
}
--
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] bridge-netfilter fixes
2006-08-23 0:10 [PATCH 0/4] bridge-netfilter fixes shemminger
` (3 preceding siblings ...)
2006-08-23 0:10 ` [PATCH 4/4] bridge-netfilter: debug message fixes shemminger
@ 2006-08-27 3:27 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2006-08-27 3:27 UTC (permalink / raw)
To: shemminger; +Cc: netdev
From: shemminger@osdl.org
Date: Tue, 22 Aug 2006 17:10:50 -0700
> This set of patches fixes issues with bridge netfilter code.
> First patch is for 2.6.18 and fixes a crash. Later patches
> could be deferred, they just cleanup the style, usage, etc.
What a nasty bug.
I'll push the bug fix to Linus for 2.6.18 right now, and
after he eats that I'll rebase net-2.6.19 then put your
other 3 patches on top.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread