[ewg] [PATCH] IB/ipoib: avoid WARN_ON on NULL path->ah

[Eli Cohen]

Commit ee1e2c82c245a5fb2864e9dbcdaab3390fde3fcc introduced an
optimization on path flushing. This caused a new possible scenario in
which unicast_arp_send triggers path query which could fail, causing
path->ah to become NULL. A successive successfull path query will then
trigger WARN_ON() in path_rec_completion(). This fix requires old_ah
to differ from NULL as a prerequsite to trigger the WARN_ON().
Moreover, that commit also allowed path resolution to be triggered for
an invalid path; if that path resolution failed, old_ah would be freed
outside priv->lock violating the assumption that dropping references
inside the lock are guaranteed not to reach zero reference.

Signed-off-by: Eli Cohen <eli at mellanox.co.il>
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 7e9e218..b1b425f 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -442,7 +442,7 @@ static void path_rec_completion(int status,
 
 		list_for_each_entry_safe(neigh, tn, &path->neigh_list, list) {
 			if (neigh->ah) {
-				WARN_ON(neigh->ah != old_ah);
+				WARN_ON(old_ah && neigh->ah != old_ah);
 				/*
 				 * Dropping the ah reference inside
 				 * priv->lock is safe here, because we
@@ -475,6 +475,20 @@ static void path_rec_completion(int status,
 				__skb_queue_tail(&skqueue, skb);
 		}
 		path->valid = 1;
+	} else {
+		list_for_each_entry_safe(neigh, tn, &path->neigh_list, list) {
+			if (neigh->ah) {
+				/*
+				 * Dropping the ah reference inside
+				 * priv->lock is safe here, because we
+				 * will hold one more reference from
+				 * the original value of path->ah (ie
+				 * old_ah).
+				 */
+				ipoib_put_ah(neigh->ah);
+				neigh->ah = NULL;
+			}
+		}
 	}
 
 	path->query = NULL;
-- 
1.6.0.2