FromDevice: avoid kernel panic on device status change.

When device status changes (e.g. link bounce), if dev->br_port is not null linux bridge's br_device_event will refer to dev->br_port. But dev->br_port is a invalid pointer to linux bridge as it's fake_bridge which was allocated by FromDevice. To make linux bridge happy, set br_port to null with early notification handler and set it again with late notification handler again. Note: this fix is under *not* ideal assumption as it assumes br_device_notifier's prioriy is higher than INT_MIN and less than INT_MAX. Signed-off-by: Joonwoo Park <[email protected]>
tbarbette · Jun 12, 2011 · b615c26 · b615c26
1 parent c5e77eb
commit b615c26
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 10 deletions.
diff --git a/elements/linuxmodule/anydevice.cc b/elements/linuxmodule/anydevice.cc
@@ -124,6 +124,8 @@ void
 AnyDevice::alter_from_device(int delta)
 {
 #if !HAVE_CLICK_KERNEL && (defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)) && LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 24)
+    if (!_dev)
+	return;
     fake_bridge *fb = reinterpret_cast<fake_bridge *>(_dev->br_port);
     if (fb && fb->magic != fake_bridge::click_magic) {
 	printk("<1>%s: appears to be owned by the bridge module!", _devname.c_str());

diff --git a/elements/linuxmodule/anydevice.hh b/elements/linuxmodule/anydevice.hh
@@ -101,6 +101,7 @@ class AnyDevice : public Element { public:
     };
     void set_device(net_device *dev, AnyDeviceMap *map, int flags);
     void clear_device(AnyDeviceMap *map, int flags);
+    void alter_from_device(int delta);
 
     static inline net_device *get_by_name(const char *name) {
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 24)
@@ -140,7 +141,6 @@ class AnyDevice : public Element { public:
     HandlerCall *_down_call;
 
     void alter_promiscuity(int delta);
-    void alter_from_device(int delta);
 
     friend class AnyDeviceMap;
 

diff --git a/elements/linuxmodule/fromdevice.cc b/elements/linuxmodule/fromdevice.cc
@@ -36,7 +36,8 @@ static int registered_readers;
 #if HAVE_CLICK_KERNEL
 static struct notifier_block packet_notifier;
 #endif
-static struct notifier_block device_notifier;
+static struct notifier_block device_notifier_early;
+static struct notifier_block device_notifier_late;
 
 #if !HAVE_CLICK_KERNEL && (defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE))
 # define CLICK_FROMDEVICE_USE_BRIDGE 1
@@ -58,7 +59,8 @@ static int packet_notifier_hook(struct notifier_block *nb, unsigned long val, vo
 static struct sk_buff *click_br_handle_frame_hook(struct net_bridge_port *p, struct sk_buff *skb);
 static struct sk_buff *(*real_br_handle_frame_hook)(struct net_bridge_port *p, struct sk_buff *skb);
 #endif
-static int device_notifier_hook(struct notifier_block *nb, unsigned long val, void *v);
+static int device_notifier_hook_early(struct notifier_block *nb, unsigned long val, void *v);
+static int device_notifier_hook_late(struct notifier_block *nb, unsigned long val, void *v);
 }
 
 void
@@ -70,10 +72,15 @@ FromDevice::static_initialize()
     packet_notifier.priority = 1;
     packet_notifier.next = 0;
 #endif
-    device_notifier.notifier_call = device_notifier_hook;
-    device_notifier.priority = 1;
-    device_notifier.next = 0;
-    register_netdevice_notifier(&device_notifier);
+    device_notifier_early.notifier_call = device_notifier_hook_early;
+    device_notifier_early.priority = INT_MAX;
+    device_notifier_early.next = 0;
+    register_netdevice_notifier(&device_notifier_early);
+
+    device_notifier_late.notifier_call = device_notifier_hook_late;
+    device_notifier_late.priority = INT_MIN;
+    device_notifier_late.next = 0;
+    register_netdevice_notifier(&device_notifier_late);
 }
 
 void
@@ -86,7 +93,8 @@ FromDevice::static_cleanup()
     if (br_handle_frame_hook == click_br_handle_frame_hook)
 	br_handle_frame_hook = real_br_handle_frame_hook;
 #endif
-    unregister_netdevice_notifier(&device_notifier);
+    unregister_netdevice_notifier(&device_notifier_late);
+    unregister_netdevice_notifier(&device_notifier_early);
 }
 
 FromDevice::FromDevice()
@@ -260,6 +268,7 @@ click_br_handle_frame_hook(struct net_bridge_port *p, struct sk_buff *skb)
     int stolen = 0;
     FromDevice *fd = 0;
     unsigned long lock_flags;
+
     from_device_map.lock(false, lock_flags);
     while (stolen == 0 && (fd = (FromDevice *)from_device_map.lookup(skb->dev, fd)))
 	stolen = fd->got_skb(skb);
@@ -274,7 +283,31 @@ click_br_handle_frame_hook(struct net_bridge_port *p, struct sk_buff *skb)
 #endif
 
 static int
-device_notifier_hook(struct notifier_block *nb, unsigned long flags, void *v)
+device_notifier_hook_early(struct notifier_block *nb, unsigned long flags, void *v)
+{
+    unsigned long lock_flags;
+    net_device* dev = (net_device*)v;
+    AnyDevice *es[8];
+    int i, nes;
+
+#ifdef NETDEV_GOING_DOWN
+    if (flags == NETDEV_GOING_DOWN)
+	flags = NETDEV_DOWN;
+#endif
+    if (flags == NETDEV_DOWN || flags == NETDEV_UP || flags == NETDEV_CHANGE) {
+	bool exists = (flags != NETDEV_UP);
+	from_device_map.lock(true, lock_flags);
+	nes = from_device_map.lookup_all(dev, exists, es, 8);
+	for (i = 0; i < nes; i++)
+	    ((FromDevice*)(es[i]))->alter_from_device(-1);
+	from_device_map.unlock(true, lock_flags);
+    }
+
+    return 0;
+}
+
+static int
+device_notifier_hook_late(struct notifier_block *nb, unsigned long flags, void *v)
 {
 #ifdef NETDEV_GOING_DOWN
     if (flags == NETDEV_GOING_DOWN)
@@ -287,8 +320,10 @@ device_notifier_hook(struct notifier_block *nb, unsigned long flags, void *v)
 	from_device_map.lock(true, lock_flags);
 	AnyDevice *es[8];
 	int nes = from_device_map.lookup_all(dev, exists, es, 8);
-	for (int i = 0; i < nes; i++)
+	for (int i = 0; i < nes; i++) {
+	    ((FromDevice*)(es[i]))->alter_from_device(1);
 	    ((FromDevice*)(es[i]))->set_device(flags == NETDEV_DOWN ? 0 : dev, &from_device_map, AnyDevice::anydev_change | AnyDevice::anydev_from_device);
+	}
 	from_device_map.unlock(true, lock_flags);
     }
     return 0;