From 1cae1a0ad2fe88499f2fd847d50b101d1985926b Mon Sep 17 00:00:00 2001 From: Wengang Wang Date: Mon, 31 Oct 2016 12:10:44 -0700 Subject: [PATCH] mlx4: avoid multiple free on id_map_ent orabug: 25022868 quickref: 24486198 (uek2) 25022856 (uek3) Double free is found on id_map_ent. Existing code makes another queue_delayed_work regardless if the worker was successfully canceled or not. In case it's not, means the worker routine already started to run, and then the later queued work will do the same work against the same id_map_ent stucture. The worker routine actually does cleanup/free on the structure, so the 2nd run of it is dengerous. Fix is that we check if we successfully canceled previously queued work, if that's not canceled, we don't queue the work again on the same structure. Signed-off-by: Wengang Wang Reviewed-by: Venkat Venkatsubra Reviewed-by: Avinash Repaka --- drivers/infiniband/hw/mlx4/cm.c | 52 ++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c index ec5635f574af9..8fe22c1d19dbe 100644 --- a/drivers/infiniband/hw/mlx4/cm.c +++ b/drivers/infiniband/hw/mlx4/cm.c @@ -243,6 +243,34 @@ static void sl_id_map_add(struct ib_device *ibdev, struct id_map_entry *new) rb_insert_color(&new->node, sl_id_map); } +/* try to reschedule the delayed work + * if not scheduled before, schedule it; otherwise, try to cancel it first and + * reschedule on the cuccess of cancellation. + * + * sriov->going_down_lock & sriov->id_map_lock are required by caller + * + * returns 1 when (re)scheduled successfully; 0 otherwise + */ +static int __try_reschedule(struct id_map_entry *ent) +{ + if (!ent->scheduled_delete) { + ent->scheduled_delete = 1; + schedule_delayed_work(&ent->timeout, CM_CLEANUP_CACHE_TIMEOUT); + return 1; + } + + /* try to cancel delayed work */ + if (cancel_delayed_work(&ent->timeout)) { + /* work was successfully cancelled, work not running, + * it's safe to queue another one */ + schedule_delayed_work(&ent->timeout, CM_CLEANUP_CACHE_TIMEOUT); + return 1; + } + + /* the timeout() work maybe running, don't queue another one */ + return 0; +} + static void schedule_delayed(struct ib_device *ibdev, struct id_map_entry *id) { struct mlx4_ib_sriov *sriov = &to_mdev(ibdev)->sriov; @@ -251,10 +279,8 @@ static void schedule_delayed(struct ib_device *ibdev, struct id_map_entry *id) spin_lock(&sriov->id_map_lock); spin_lock_irqsave(&sriov->going_down_lock, flags); /*make sure that there is no schedule inside the scheduled work.*/ - if (!sriov->is_going_down) { - id->scheduled_delete = 1; - schedule_delayed_work(&id->timeout, CM_CLEANUP_CACHE_TIMEOUT); - } + if (!sriov->is_going_down) + __try_reschedule(id); spin_unlock_irqrestore(&sriov->going_down_lock, flags); spin_unlock(&sriov->id_map_lock); } @@ -318,14 +344,14 @@ id_map_get(struct ib_device *ibdev, int *pv_cm_id, int sl_cm_id, int slave_id, ent = (struct id_map_entry *)idr_find(&sriov->pv_id_table, *pv_cm_id); if (ent && sched_or_cancel) { if (sched_or_cancel == ID_SCHED_DELETE) { - cancel_delayed_work(&ent->timeout); - ent->scheduled_delete = 1; - schedule_delayed_work(&ent->timeout, - CM_CLEANUP_CACHE_TIMEOUT); + if (!__try_reschedule(ent)) + ent = NULL; } if (sched_or_cancel == ID_CANCEL_DELETE) { - ent->scheduled_delete = 0; - cancel_delayed_work(&ent->timeout); + if (cancel_delayed_work(&ent->timeout)) + ent->scheduled_delete = 0; + else + ent = NULL; } } spin_unlock(&sriov->id_map_lock); @@ -446,7 +472,7 @@ void mlx4_ib_cm_paravirt_clean(struct mlx4_ib_dev *dev, int slave) struct rb_root *sl_id_map = &sriov->sl_id_map; struct list_head lh; struct rb_node *nd; - int need_flush = 1; + int no_flush = 1; struct id_map_entry *map, *tmp_map; /* cancel all delayed work queue entries */ INIT_LIST_HEAD(&lh); @@ -454,13 +480,13 @@ void mlx4_ib_cm_paravirt_clean(struct mlx4_ib_dev *dev, int slave) list_for_each_entry_safe(map, tmp_map, &dev->sriov.cm_list, list) { if (slave < 0 || slave == map->slave_id) { if (map->scheduled_delete) - need_flush &= !!cancel_delayed_work(&map->timeout); + no_flush &= !!cancel_delayed_work(&map->timeout); } } spin_unlock(&sriov->id_map_lock); - if (!need_flush) + if (!no_flush) flush_scheduled_work(); /* make sure all timers were flushed */ /* now, remove all leftover entries from databases*/ -- 2.50.1