From 6df21d1f2965398ac71606dcc2ce25090134da16 Mon Sep 17 00:00:00 2001 From: bings Date: Tue, 19 Feb 2019 17:59:45 +0800 Subject: [PATCH] qcacmn: Always free nol timer before free pdev When wlan target asserts, driver will trigger shutdown and reinit to do recovery. In shutdown, when pdev is freed and dfs_detach is called, dfs->dfs_nol should be freed. If at the same time dfs_remove_from_nol is triggered in another CPU, a race condition may happen as follows: NOL is moved from dfs->dfs_nol to dfs->dfs_nol_free_list in function dfs_remove_from_nol, then dfs_detach sets dfs->dfs_pdev_obj to NULL without freeing the corresponding NOL timer. QDF_ASSERT happens at this time if the function dfs_remove_from_nol continues to execute. To be specific: The timer expiry function dfs_remove_from_nol calls several umac functions e.g. dfs_mlme_nol_timeout_notification which needs dfs->dfs_pdev_obj as an argument. However, the dfs->dfs_pdev_obj is set to NULL by wlan_dfs_pdev_obj_destroy_notification function. Even though before dfs->dfs_pdev_obj is set to NULL, the wlan_dfs_pdev_obj_destroy_notification waits for the workqueue qdf_flush_work(&dfs->dfs_nol_elem_free_work) which in turn waits for the timer epiry function, scheduling of the workqueue seems to have happened after qdf_flush_work(&dfs->dfs_nol_elem_free_work) in this case. If qdf_flush_work happens before the workqueue is scheduled then qdf_flush_work does not wait for the workqueue and therefore we do not wait for the timer expiry function before setting dfs->dfs_pdev_obj to NULL. To fix the race condition, trigger dfs->dfs_nol_elem_free_work immediately after dfs_nol_delete. Change-Id: I9fc07231b07139ebe794fc7cc94d2a8ab307c69f CRs-Fixed: 2400959 --- umac/dfs/core/src/dfs.h | 6 ++++++ umac/dfs/core/src/misc/dfs_nol.c | 7 ------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/umac/dfs/core/src/dfs.h b/umac/dfs/core/src/dfs.h index e12ab55220..da97192197 100644 --- a/umac/dfs/core/src/dfs.h +++ b/umac/dfs/core/src/dfs.h @@ -338,10 +338,16 @@ WLAN_DFSNOL_UNLOCK(dfs); \ } while (0) +/* + * Free the NOL element in a thread. This is to avoid freeing the + * timer object from within timer callback function . The nol element + * contains the timer Object. + */ #define DFS_NOL_DELETE_CHAN_LOCKED(dfs, freq, chwidth) \ do { \ WLAN_DFSNOL_LOCK(dfs); \ dfs_nol_delete(dfs, freq, chwidth); \ + qdf_sched_work(NULL, &dfs->dfs_nol_elem_free_work); \ WLAN_DFSNOL_UNLOCK(dfs); \ } while (0) diff --git a/umac/dfs/core/src/misc/dfs_nol.c b/umac/dfs/core/src/misc/dfs_nol.c index 5a81f5c4dc..92bc2439b1 100644 --- a/umac/dfs/core/src/misc/dfs_nol.c +++ b/umac/dfs/core/src/misc/dfs_nol.c @@ -254,13 +254,6 @@ static os_timer_func(dfs_remove_from_nol) utils_dfs_reg_update_nol_ch(dfs->dfs_pdev_obj, &chan, 1, DFS_NOL_RESET); utils_dfs_save_nol(dfs->dfs_pdev_obj); - - /* - * Free the NOL element in a thread. This is to avoid freeing the - * timer object from within timer callback function . The nol element - * contains the timer Object. - */ - qdf_sched_work(NULL, &dfs->dfs_nol_elem_free_work); } void dfs_print_nol(struct wlan_dfs *dfs)