Skip to content

Commit 49b8e74

Browse files
jf6brolandd
authored andcommitted
IPoIB: Fix race in deleting ipoib_neigh entries
In several places, this snippet is used when removing neigh entries: list_del(&neigh->list); ipoib_neigh_free(neigh); The list_del() removes neigh from the associated struct ipoib_path, while ipoib_neigh_free() removes neigh from the device's neigh entry lookup table. Both of these operations are protected by the priv->lock spinlock. The table however is also protected via RCU, and so naturally the lock is not held when doing reads. This leads to a race condition, in which a thread may successfully look up a neigh entry that has already been deleted from neigh->list. Since the previous deletion will have marked the entry with poison, a second list_del() on the object will cause a panic: CyanogenMod#5 [ffff8802338c3c70] general_protection at ffffffff815108c5 [exception RIP: list_del+16] RIP: ffffffff81289020 RSP: ffff8802338c3d20 RFLAGS: 00010082 RAX: dead000000200200 RBX: ffff880433e60c88 RCX: 0000000000009e6c RDX: 0000000000000246 RSI: ffff8806012ca298 RDI: ffff880433e60c88 RBP: ffff8802338c3d30 R8: ffff8806012ca2e8 R9: 00000000ffffffff R10: 0000000000000001 R11: 0000000000000000 R12: ffff8804346b2020 R13: ffff88032a3e7540 R14: ffff8804346b26e0 R15: 0000000000000246 ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0000 coolya#6 [ffff8802338c3d38] ipoib_cm_tx_handler at ffffffffa066fe0a [ib_ipoib] coolya#7 [ffff8802338c3d98] cm_process_work at ffffffffa05149a7 [ib_cm] coolya#8 [ffff8802338c3de8] cm_work_handler at ffffffffa05161aa [ib_cm] coolya#9 [ffff8802338c3e38] worker_thread at ffffffff81090e10 coolya#10 [ffff8802338c3ee8] kthread at ffffffff81096c66 coolya#11 [ffff8802338c3f48] kernel_thread at ffffffff8100c0ca We move the list_del() into ipoib_neigh_free(), so that deletion happens only once, after the entry has been successfully removed from the lookup table. This same behavior is already used in ipoib_del_neighs_by_gid() and __ipoib_reap_neigh(). Signed-off-by: Jim Foraker <foraker1@llnl.gov> Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com> Reviewed-by: Jack Wang <jinpu.wang@profitbricks.com> Reviewed-by: Shlomo Pongratz <shlomop@mellanox.com> Signed-off-by: Roland Dreier <roland@purestorage.com>
1 parent c095ba7 commit 49b8e74

2 files changed

Lines changed: 3 additions & 9 deletions

File tree

drivers/infiniband/ulp/ipoib/ipoib_cm.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,6 @@ void ipoib_cm_handle_tx_wc(struct net_device *dev, struct ib_wc *wc)
817817

818818
if (neigh) {
819819
neigh->cm = NULL;
820-
list_del(&neigh->list);
821820
ipoib_neigh_free(neigh);
822821

823822
tx->neigh = NULL;
@@ -1234,7 +1233,6 @@ static int ipoib_cm_tx_handler(struct ib_cm_id *cm_id,
12341233

12351234
if (neigh) {
12361235
neigh->cm = NULL;
1237-
list_del(&neigh->list);
12381236
ipoib_neigh_free(neigh);
12391237

12401238
tx->neigh = NULL;
@@ -1325,7 +1323,6 @@ static void ipoib_cm_tx_start(struct work_struct *work)
13251323
neigh = p->neigh;
13261324
if (neigh) {
13271325
neigh->cm = NULL;
1328-
list_del(&neigh->list);
13291326
ipoib_neigh_free(neigh);
13301327
}
13311328
list_del(&p->list);

drivers/infiniband/ulp/ipoib/ipoib_main.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,6 @@ static void path_rec_completion(int status,
493493
path,
494494
neigh));
495495
if (!ipoib_cm_get(neigh)) {
496-
list_del(&neigh->list);
497496
ipoib_neigh_free(neigh);
498497
continue;
499498
}
@@ -618,7 +617,6 @@ static void neigh_add_path(struct sk_buff *skb, u8 *daddr,
618617
if (!ipoib_cm_get(neigh))
619618
ipoib_cm_set(neigh, ipoib_cm_create_tx(dev, path, neigh));
620619
if (!ipoib_cm_get(neigh)) {
621-
list_del(&neigh->list);
622620
ipoib_neigh_free(neigh);
623621
goto err_drop;
624622
}
@@ -639,7 +637,7 @@ static void neigh_add_path(struct sk_buff *skb, u8 *daddr,
639637
neigh->ah = NULL;
640638

641639
if (!path->query && path_rec_start(dev, path))
642-
goto err_list;
640+
goto err_path;
643641

644642
__skb_queue_tail(&neigh->queue, skb);
645643
}
@@ -648,9 +646,6 @@ static void neigh_add_path(struct sk_buff *skb, u8 *daddr,
648646
ipoib_neigh_put(neigh);
649647
return;
650648

651-
err_list:
652-
list_del(&neigh->list);
653-
654649
err_path:
655650
ipoib_neigh_free(neigh);
656651
err_drop:
@@ -1098,6 +1093,8 @@ void ipoib_neigh_free(struct ipoib_neigh *neigh)
10981093
rcu_assign_pointer(*np,
10991094
rcu_dereference_protected(neigh->hnext,
11001095
lockdep_is_held(&priv->lock)));
1096+
/* remove from parent list */
1097+
list_del(&neigh->list);
11011098
call_rcu(&neigh->rcu, ipoib_neigh_reclaim);
11021099
return;
11031100
} else {

0 commit comments

Comments
 (0)