IPoIB: Close race in setting mcast->ah

ipoib_mcast_send() tests mcast->ah twice.  If this value is changed
between these two points, we leak an skb.  However,
ipoib_mcast_join_finish() sets mcast->ah with no locking, so it could
race against ipoib_mcast_send().

As a solution, take priv->lock around assignment to mcast->ah thus
making sure ipoib_mcast_send() (which also takes priv->lock) is not in
flight.

Signed-off-by: Eli Cohen <eli@mellanox.co.il>
Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>
Signed-off-by: Roland Dreier <rolandd@cisco.com>

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index a2408d7ec5986cb3e69b3988f9a6c5a35fbf118f..e5dc2a03453081a3f1af7c6bb5e8a4e364761870 100644 (file)
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -213,6 +213,7 @@ static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast,
 {
        struct net_device *dev = mcast->dev;
        struct ipoib_dev_priv *priv = netdev_priv(dev);
+       struct ipoib_ah *ah;
        int ret;
 
        mcast->mcmember = *mcmember;
@@ -269,8 +270,8 @@ static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast,
                                av.static_rate, priv->local_rate,
                                ib_sa_rate_enum_to_int(mcast->mcmember.rate));
 
-               mcast->ah = ipoib_create_ah(dev, priv->pd, &av);
-               if (!mcast->ah) {
+               ah = ipoib_create_ah(dev, priv->pd, &av);
+               if (!ah) {
                        ipoib_warn(priv, "ib_address_create failed\n");
                } else {
                        ipoib_dbg_mcast(priv, "MGID " IPOIB_GID_FMT
@@ -280,6 +281,10 @@ static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast,
                                        be16_to_cpu(mcast->mcmember.mlid),
                                        mcast->mcmember.sl);
                }
+
+               spin_lock_irq(&priv->lock);
+               mcast->ah = ah;
+               spin_unlock_irq(&priv->lock);
        }
 
        /* actually send any queued packets */