[ofa-general] [PATCH] ipoib: racing uses of ipoib_neigh in CM

[akepner at sgi.com]

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 <akepner at sgi.com>
---

 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;