[PATCH 1/3] bridge: fix for RCU and deadlock on device removal

2006-02-06 Thread Stephen Hemminger
Change Bridge receive path to correctly handle RCU removal of device
from bridge.  Also fixes deadlock between carrier_check and del_nbp.
This replaces the previous deleted flag fix.

Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>


--- br-2.6.orig/net/bridge/br_input.c
+++ br-2.6/net/bridge/br_input.c
@@ -45,18 +45,20 @@ static void br_pass_frame_up(struct net_
 int br_handle_frame_finish(struct sk_buff *skb)
 {
const unsigned char *dest = eth_hdr(skb)->h_dest;
-   struct net_bridge_port *p = skb->dev->br_port;
-   struct net_bridge *br = p->br;
+   struct net_bridge_port *p = rcu_dereference(skb->dev->br_port);
+   struct net_bridge *br;
struct net_bridge_fdb_entry *dst;
int passedup = 0;
 
+   if (!p || p->state == BR_STATE_DISABLED)
+   goto drop;
+
/* insert into forwarding database after filtering to avoid spoofing */
-   br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
+   br = p->br;
+   br_fdb_update(br, p, eth_hdr(skb)->h_source);
 
-   if (p->state == BR_STATE_LEARNING) {
-   kfree_skb(skb);
-   goto out;
-   }
+   if (p->state == BR_STATE_LEARNING)
+   goto drop;
 
if (br->dev->flags & IFF_PROMISC) {
struct sk_buff *skb2;
@@ -93,6 +95,9 @@ int br_handle_frame_finish(struct sk_buf
 
 out:
return 0;
+drop:
+   kfree_skb(skb);
+   goto out;
 }
 
 /*
--- br-2.6.orig/net/bridge/br_private.h
+++ br-2.6/net/bridge/br_private.h
@@ -68,7 +68,6 @@ struct net_bridge_port
/* STP */
u8  priority;
u8  state;
-   u8  deleted;
u16 port_no;
unsigned char   topology_change_ack;
unsigned char   config_pending;
--- br-2.6.orig/net/bridge/br_stp_bpdu.c
+++ br-2.6/net/bridge/br_stp_bpdu.c
@@ -133,29 +133,35 @@ void br_send_tcn_bpdu(struct net_bridge_
 
 static const unsigned char header[6] = {0x42, 0x42, 0x03, 0x00, 0x00, 0x00};
 
-/* NO locks */
+/* NO locks, but rcu_read_lock (preempt_disabled)  */
 int br_stp_handle_bpdu(struct sk_buff *skb)
 {
-   struct net_bridge_port *p = skb->dev->br_port;
-   struct net_bridge *br = p->br;
+   struct net_bridge_port *p = rcu_dereference(skb->dev->br_port);
+   struct net_bridge *br;
unsigned char *buf;
 
+   if (!p)
+   goto err;
+
+   br = p->br;
+   spin_lock(&br->lock);
+
+   if (p->state == BR_STATE_DISABLED || !(br->dev->flags & IFF_UP))
+   goto out;
+
/* insert into forwarding database after filtering to avoid spoofing */
-   br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
+   br_fdb_update(br, p, eth_hdr(skb)->h_source);
+
+   if (!br->stp_enabled)
+   goto out;
 
/* need at least the 802 and STP headers */
if (!pskb_may_pull(skb, sizeof(header)+1) ||
memcmp(skb->data, header, sizeof(header)))
-   goto err;
+   goto out;
 
buf = skb_pull(skb, sizeof(header));
 
-   spin_lock_bh(&br->lock);
-   if (p->state == BR_STATE_DISABLED 
-   || !(br->dev->flags & IFF_UP)
-   || !br->stp_enabled)
-   goto out;
-
if (buf[0] == BPDU_TYPE_CONFIG) {
struct br_config_bpdu bpdu;
 
@@ -201,7 +207,7 @@ int br_stp_handle_bpdu(struct sk_buff *s
br_received_tcn_bpdu(p);
}
  out:
-   spin_unlock_bh(&br->lock);
+   spin_unlock(&br->lock);
  err:
kfree_skb(skb);
return 0;
--- br-2.6.orig/net/bridge/br_if.c
+++ br-2.6/net/bridge/br_if.c
@@ -79,9 +79,14 @@ static int port_cost(struct net_device *
  */
 static void port_carrier_check(void *arg)
 {
-   struct net_bridge_port *p = arg;
+   struct net_device *dev = arg;
+   struct net_bridge_port *p;
 
rtnl_lock();
+   p = dev->br_port;
+   if (!p)
+   goto done;
+
if (netif_carrier_ok(p->dev)) {
u32 cost = port_cost(p->dev);
 
@@ -97,6 +102,7 @@ static void port_carrier_check(void *arg
br_stp_disable_port(p);
spin_unlock_bh(&p->br->lock);
}
+done:
rtnl_unlock();
 }
 
@@ -104,7 +110,6 @@ static void destroy_nbp(struct net_bridg
 {
struct net_device *dev = p->dev;
 
-   dev->br_port = NULL;
p->br = NULL;
p->dev = NULL;
dev_put(dev);
@@ -133,24 +138,20 @@ static void del_nbp(struct net_bridge_po
struct net_bridge *br = p->br;
struct net_device *dev = p->dev;
 
-   /* Race between RTNL notify and RCU callback */
-   if (p->deleted)
-   return;
-
dev_set_promiscuity(dev, -1);
 
cancel_delayed_work(&p->carrier_check);
-   flush_scheduled_work();
 
spin_lock_bh(&br->lock);
br_stp_disable_port(p);
-   p->de

Re: [PATCH 1/3] bridge: fix for RCU and deadlock on device removal

2006-02-09 Thread David S. Miller
From: Stephen Hemminger <[EMAIL PROTECTED]>
Date: Mon, 06 Feb 2006 14:27:33 -0800

> Change Bridge receive path to correctly handle RCU removal of device
> from bridge.  Also fixes deadlock between carrier_check and del_nbp.
> This replaces the previous deleted flag fix.
> 
> Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>

Applied to net-2.6, thanks Stephen.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html