diff options
author | Evgeniy Polyakov <johnpol@2ka.mipt.ru> | 2007-08-25 02:36:29 -0400 |
---|---|---|
committer | David S. Miller <davem@sunset.davemloft.net> | 2007-08-26 21:35:47 -0400 |
commit | e7c243c925f6d9dcb898504ff24d6650b5cbb3b1 (patch) | |
tree | f06ae59e206e4876b0326c65811f496a8b1f4bdc /net/bridge | |
parent | 7c8347a91dbbb723d8ed106ec817dabac97f2bbc (diff) |
[VLAN/BRIDGE]: Fix "skb_pull_rcsum - Fatal exception in interrupt"
I tried to preserve bridging code as it was before, but logic is quite
strange - I think we should free skb on error, since it is already
unshared and thus will just leak.
Herbert Xu states:
> + if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL)
> + goto out;
If this happens it'll be a double-free on skb since we'll
return NF_DROP which makes the caller free it too.
We could return NF_STOLEN to prevent that but I'm not sure
whether that's correct netfilter semantics. Patrick, could
you please make a call on this?
Patrick McHardy states:
NF_STOLEN should work fine here.
Signed-off-by: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/bridge')
-rw-r--r-- | net/bridge/br_netfilter.c | 12 |
1 files changed, 7 insertions, 5 deletions
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c index fa779874b9dd..3ee2022928e3 100644 --- a/net/bridge/br_netfilter.c +++ b/net/bridge/br_netfilter.c | |||
@@ -509,8 +509,14 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff **pskb, | |||
509 | int (*okfn)(struct sk_buff *)) | 509 | int (*okfn)(struct sk_buff *)) |
510 | { | 510 | { |
511 | struct iphdr *iph; | 511 | struct iphdr *iph; |
512 | __u32 len; | ||
513 | struct sk_buff *skb = *pskb; | 512 | struct sk_buff *skb = *pskb; |
513 | __u32 len = nf_bridge_encap_header_len(skb); | ||
514 | |||
515 | if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL) | ||
516 | return NF_STOLEN; | ||
517 | |||
518 | if (unlikely(!pskb_may_pull(skb, len))) | ||
519 | goto out; | ||
514 | 520 | ||
515 | if (skb->protocol == htons(ETH_P_IPV6) || IS_VLAN_IPV6(skb) || | 521 | if (skb->protocol == htons(ETH_P_IPV6) || IS_VLAN_IPV6(skb) || |
516 | IS_PPPOE_IPV6(skb)) { | 522 | IS_PPPOE_IPV6(skb)) { |
@@ -518,8 +524,6 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff **pskb, | |||
518 | if (!brnf_call_ip6tables) | 524 | if (!brnf_call_ip6tables) |
519 | return NF_ACCEPT; | 525 | return NF_ACCEPT; |
520 | #endif | 526 | #endif |
521 | if ((skb = skb_share_check(*pskb, GFP_ATOMIC)) == NULL) | ||
522 | goto out; | ||
523 | nf_bridge_pull_encap_header_rcsum(skb); | 527 | nf_bridge_pull_encap_header_rcsum(skb); |
524 | return br_nf_pre_routing_ipv6(hook, skb, in, out, okfn); | 528 | return br_nf_pre_routing_ipv6(hook, skb, in, out, okfn); |
525 | } | 529 | } |
@@ -532,8 +536,6 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff **pskb, | |||
532 | !IS_PPPOE_IP(skb)) | 536 | !IS_PPPOE_IP(skb)) |
533 | return NF_ACCEPT; | 537 | return NF_ACCEPT; |
534 | 538 | ||
535 | if ((skb = skb_share_check(*pskb, GFP_ATOMIC)) == NULL) | ||
536 | goto out; | ||
537 | nf_bridge_pull_encap_header_rcsum(skb); | 539 | nf_bridge_pull_encap_header_rcsum(skb); |
538 | 540 | ||
539 | if (!pskb_may_pull(skb, sizeof(struct iphdr))) | 541 | if (!pskb_may_pull(skb, sizeof(struct iphdr))) |