llc: Fix races between llc2 handler use and (un)registration
authorBen Hutchings <ben@decadent.org.uk>
Mon, 13 Aug 2012 02:50:55 +0000 (02:50 +0000)
committerDavid S. Miller <davem@davemloft.net>
Tue, 14 Aug 2012 23:52:02 +0000 (16:52 -0700)
When registering the handlers, any state they rely on must be
completely initialised first.  When unregistering, we must wait until
they are definitely no longer running.  llc_rcv() must also avoid
reading the handler pointers again after checking for NULL.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/llc/llc_input.c
net/llc/llc_station.c

index e32cab44ea959d8f49781d46580b4204afb7e3cb..dd3e83328ad544d3183233da61fa40289975de51 100644 (file)
@@ -42,6 +42,7 @@ static void (*llc_type_handlers[2])(struct llc_sap *sap,
 void llc_add_pack(int type, void (*handler)(struct llc_sap *sap,
                                            struct sk_buff *skb))
 {
+       smp_wmb(); /* ensure initialisation is complete before it's called */
        if (type == LLC_DEST_SAP || type == LLC_DEST_CONN)
                llc_type_handlers[type - 1] = handler;
 }
@@ -50,11 +51,19 @@ void llc_remove_pack(int type)
 {
        if (type == LLC_DEST_SAP || type == LLC_DEST_CONN)
                llc_type_handlers[type - 1] = NULL;
+       synchronize_net();
 }
 
 void llc_set_station_handler(void (*handler)(struct sk_buff *skb))
 {
+       /* Ensure initialisation is complete before it's called */
+       if (handler)
+               smp_wmb();
+
        llc_station_handler = handler;
+
+       if (!handler)
+               synchronize_net();
 }
 
 /**
@@ -150,6 +159,8 @@ int llc_rcv(struct sk_buff *skb, struct net_device *dev,
        int dest;
        int (*rcv)(struct sk_buff *, struct net_device *,
                   struct packet_type *, struct net_device *);
+       void (*sta_handler)(struct sk_buff *skb);
+       void (*sap_handler)(struct llc_sap *sap, struct sk_buff *skb);
 
        if (!net_eq(dev_net(dev), &init_net))
                goto drop;
@@ -182,7 +193,8 @@ int llc_rcv(struct sk_buff *skb, struct net_device *dev,
         */
        rcv = rcu_dereference(sap->rcv_func);
        dest = llc_pdu_type(skb);
-       if (unlikely(!dest || !llc_type_handlers[dest - 1])) {
+       sap_handler = dest ? ACCESS_ONCE(llc_type_handlers[dest - 1]) : NULL;
+       if (unlikely(!sap_handler)) {
                if (rcv)
                        rcv(skb, dev, pt, orig_dev);
                else
@@ -193,7 +205,7 @@ int llc_rcv(struct sk_buff *skb, struct net_device *dev,
                        if (cskb)
                                rcv(cskb, dev, pt, orig_dev);
                }
-               llc_type_handlers[dest - 1](sap, skb);
+               sap_handler(sap, skb);
        }
        llc_sap_put(sap);
 out:
@@ -202,9 +214,10 @@ drop:
        kfree_skb(skb);
        goto out;
 handle_station:
-       if (!llc_station_handler)
+       sta_handler = ACCESS_ONCE(llc_station_handler);
+       if (!sta_handler)
                goto drop;
-       llc_station_handler(skb);
+       sta_handler(skb);
        goto out;
 }
 
index bba5184fafd7966ec26fadd4a51163f07a62c0d7..b2f2bac2c2a2397b5fd6f6b79659996270b96377 100644 (file)
@@ -696,9 +696,9 @@ void __init llc_station_init(void)
                        (unsigned long)&llc_main_station);
        llc_main_station.ack_timer.expires  = jiffies +
                                                sysctl_llc_station_ack_timeout;
-       llc_set_station_handler(llc_station_rcv);
        llc_main_station.maximum_retry  = 1;
        llc_main_station.state          = LLC_STATION_STATE_UP;
+       llc_set_station_handler(llc_station_rcv);
 }
 
 void llc_station_exit(void)