There is a race condition between the neighbour cleanup code, and
some uses of the closely related ipoib_neigh structure when using
IPoIB-CM. The root cause is that the struct ipoib_neigh pointer 
that's stashed away in struct neighbour is read (and subsequently 
used) in ipoib_neigh_cleanup() without using locking that's consistent 
with other reads/writes of this pointer. (The pointer must be read 
before it can be known which lock to use, so it's difficult to 
avoid.)

Re-read the pointer in ipoib_neigh_cleanup() after the appropriate 
lock is acquired.

Signed-off-by: Arthur Kepner <[email protected]>
---

 ipoib_main.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index ab2c192..901735b 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -841,10 +841,20 @@ static void ipoib_set_mcast_list(struct net_device *dev)
 static void ipoib_neigh_cleanup(struct neighbour *n)
 {
        struct ipoib_neigh *neigh;
-       struct ipoib_dev_priv *priv = netdev_priv(n->dev);
+       struct ipoib_dev_priv *priv;
        unsigned long flags;
        struct ipoib_ah *ah = NULL;
 
+       /* 
+        * Note that the read of the neigh pointer below is not protected 
+        * by a ipoib_dev_priv->lock (since we don't yet know which device's 
+        * lock to use). Count on the fact that if ipoib_neigh_free() has 
+        * already freed the struct ipoib_neigh, to_ipoib_neigh() will 
+        * return NULL.
+        * 
+        * If to_ipoib_neigh() does not return NULL, we'll re-read neigh 
+        * under the appropriate lock below. 
+        */
        neigh = *to_ipoib_neigh(n);
        if (neigh)
                priv = netdev_priv(neigh->dev);
@@ -856,11 +866,14 @@ static void ipoib_neigh_cleanup(struct neighbour *n)
                  n->ha + 4);
 
        spin_lock_irqsave(&priv->lock, flags);
-
-       if (neigh->ah)
-               ah = neigh->ah;
-       list_del(&neigh->list);
-       ipoib_neigh_free(n->dev, neigh);
+       /* re-read neigh under priv->lock */
+       neigh = *to_ipoib_neigh(n);
+       if (neigh) {
+               if (neigh->ah)
+                       ah = neigh->ah;
+               list_del(&neigh->list);
+               ipoib_neigh_free(n->dev, neigh);
+       }
 
        spin_unlock_irqrestore(&priv->lock, flags);
 
@@ -888,7 +901,11 @@ struct ipoib_neigh *ipoib_neigh_alloc(struct neighbour 
*neighbour,
 
 void ipoib_neigh_free(struct net_device *dev, struct ipoib_neigh *neigh)
 {
+       struct ipoib_dev_priv *priv = netdev_priv(neigh->dev);
        struct sk_buff *skb;
+
+       BUG_ON(!spin_is_locked(&priv->lock));
+
        *to_ipoib_neigh(neigh->neighbour) = NULL;
        while ((skb = __skb_dequeue(&neigh->queue))) {
                ++dev->stats.tx_dropped;
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to