From 0d2eeaeb873b06964e725192df21745cad1baab9 Mon Sep 17 00:00:00 2001 From: Dustin Brown Date: Fri, 24 Mar 2017 15:21:32 -0700 Subject: [PATCH] qcacld-3.0: Do objmgr vdev destroy before sme close session Converged host driver code listens for a vdev_destroy event from object manager to release vdev related resources. Currently, vdevs are destroyed in firmware before they are destroyed by object manager, leading to a possible use-after-free situation in firmware. Reverse the order of vdev destroy, first in object manger, then in firmware, to prevent the potential use-after-free. Change-Id: I54bbede9732cecb3bec291692f452758976184c4 CRs-Fixed: 2024633 --- core/hdd/src/wlan_hdd_hostapd.c | 11 +++-- core/hdd/src/wlan_hdd_main.c | 68 +++++--------------------- core/hdd/src/wlan_hdd_nan_datapath.c | 24 ++------- core/hdd/src/wlan_hdd_object_manager.c | 33 +++++++++++-- core/hdd/src/wlan_hdd_object_manager.h | 28 ++++++++++- 5 files changed, 80 insertions(+), 84 deletions(-) diff --git a/core/hdd/src/wlan_hdd_hostapd.c b/core/hdd/src/wlan_hdd_hostapd.c index 0893b903720f..74e5a441fbba 100644 --- a/core/hdd/src/wlan_hdd_hostapd.c +++ b/core/hdd/src/wlan_hdd_hostapd.c @@ -6070,8 +6070,9 @@ error_wmm_init: hdd_softap_deinit_tx_rx(pAdapter); error_init_ap_mode: - QDF_BUG(!hdd_objmgr_release_and_destroy_vdev(pAdapter)); + QDF_BUG(!hdd_objmgr_destroy_vdev(pAdapter)); wlansap_close(sapContext); + QDF_BUG(!hdd_objmgr_release_vdev(pAdapter)); pAdapter->sessionCtx.ap.sapContext = NULL; EXIT(); @@ -6241,14 +6242,18 @@ QDF_STATUS hdd_unregister_hostapd(hdd_adapter_t *pAdapter, bool rtnl_held) if (!QDF_IS_STATUS_SUCCESS(status)) hdd_err("Failed:wlansap_stop"); + ret = hdd_objmgr_destroy_vdev(pAdapter); + if (ret) + hdd_err("objmgr vdev destroy failed"); + status = wlansap_close(sapContext); if (!QDF_IS_STATUS_SUCCESS(status)) hdd_err("Failed:WLANSAP_close"); pAdapter->sessionCtx.ap.sapContext = NULL; - ret = hdd_objmgr_release_and_destroy_vdev(pAdapter); + ret = hdd_objmgr_release_vdev(pAdapter); if (ret) - hdd_err("vdev delete failed"); + hdd_err("objmgr vdev release failed"); EXIT(); return 0; diff --git a/core/hdd/src/wlan_hdd_main.c b/core/hdd/src/wlan_hdd_main.c index 33551b49af78..2dd6edb9beee 100644 --- a/core/hdd/src/wlan_hdd_main.c +++ b/core/hdd/src/wlan_hdd_main.c @@ -2915,8 +2915,8 @@ int hdd_vdev_destroy(hdd_adapter_t *adapter) return -EINVAL; } - /* do vdev create via objmgr */ - errno = hdd_objmgr_release_and_destroy_vdev(adapter); + /* do vdev logical destroy via objmgr */ + errno = hdd_objmgr_destroy_vdev(adapter); if (errno) { hdd_err("failed to destroy objmgr vdev: %d", errno); return errno; @@ -2944,6 +2944,13 @@ int hdd_vdev_destroy(hdd_adapter_t *adapter) return -ETIMEDOUT; } + /* now that sme session is closed, allow physical vdev destroy */ + errno = hdd_objmgr_release_vdev(adapter); + if (errno) { + hdd_err("failed to release objmgr vdev: %d", errno); + return errno; + } + hdd_info("vdev destroyed successfully"); return 0; @@ -3239,7 +3246,6 @@ void hdd_deinit_adapter(hdd_context_t *hdd_ctx, hdd_adapter_t *adapter, static void hdd_cleanup_adapter(hdd_context_t *hdd_ctx, hdd_adapter_t *adapter, bool rtnl_held) { - int ret; struct net_device *pWlanDev = NULL; if (adapter) @@ -3249,10 +3255,6 @@ static void hdd_cleanup_adapter(hdd_context_t *hdd_ctx, hdd_adapter_t *adapter, return; } - ret = hdd_objmgr_release_and_destroy_vdev(adapter); - if (ret) - hdd_err("vdev delete failed"); - hdd_debugfs_exit(adapter); if (adapter->scan_info.default_scan_ies) { @@ -3851,54 +3853,6 @@ void wlan_hdd_reset_prob_rspies(hdd_adapter_t *pHostapdAdapter) } } -/** - * hdd_wait_for_sme_close_sesion() - Close and wait for SME session close - * @hdd_ctx: HDD context which is already NULL validated - * @adapter: HDD adapter which is already NULL validated - * - * Close the SME session and wait for its completion, if needed. - * - * Return: None - */ -static void hdd_wait_for_sme_close_sesion(hdd_context_t *hdd_ctx, - hdd_adapter_t *adapter) -{ - int ret; - unsigned long rc; - - if (!test_bit(SME_SESSION_OPENED, &adapter->event_flags)) { - hdd_err("session is not opened:%d", adapter->sessionId); - return; - } - - INIT_COMPLETION(adapter->session_close_comp_var); - if (QDF_STATUS_SUCCESS == - sme_close_session(hdd_ctx->hHal, adapter->sessionId, - hdd_sme_close_session_callback, - adapter)) { - /* - * Block on a completion variable. Can't wait - * forever though. - */ - rc = wait_for_completion_timeout( - &adapter->session_close_comp_var, - msecs_to_jiffies - (WLAN_WAIT_TIME_SESSIONOPENCLOSE)); - if (!rc) { - hdd_err("failure waiting for session_close_comp_var"); - if (adapter->device_mode == QDF_NDI_MODE) - hdd_ndp_session_end_handler(adapter); - clear_bit(SME_SESSION_OPENED, &adapter->event_flags); - return; - } - ret = hdd_objmgr_release_and_destroy_vdev(adapter); - if (ret) - hdd_err("vdev delete failed"); - - adapter->sessionId = HDD_SESSION_ID_INVALID; - } -} - QDF_STATUS hdd_stop_adapter(hdd_context_t *hdd_ctx, hdd_adapter_t *adapter, const bool bCloseSession) { @@ -3992,7 +3946,7 @@ QDF_STATUS hdd_stop_adapter(hdd_context_t *hdd_ctx, hdd_adapter_t *adapter, hdd_err("Error: Can't disconnect adapter"); return QDF_STATUS_E_FAILURE; } - hdd_wait_for_sme_close_sesion(hdd_ctx, adapter); + hdd_vdev_destroy(adapter); } break; @@ -4071,7 +4025,7 @@ QDF_STATUS hdd_stop_adapter(hdd_context_t *hdd_ctx, hdd_adapter_t *adapter, adapter->sessionCtx.ap.beacon = NULL; } if (true == bCloseSession) - hdd_wait_for_sme_close_sesion(hdd_ctx, adapter); + hdd_vdev_destroy(adapter); mutex_unlock(&hdd_ctx->sap_lock); break; case QDF_OCB_MODE: diff --git a/core/hdd/src/wlan_hdd_nan_datapath.c b/core/hdd/src/wlan_hdd_nan_datapath.c index eb609b95a56b..96d6c58c5512 100644 --- a/core/hdd/src/wlan_hdd_nan_datapath.c +++ b/core/hdd/src/wlan_hdd_nan_datapath.c @@ -113,9 +113,8 @@ void hdd_nan_datapath_target_config(hdd_context_t *hdd_ctx, */ static int hdd_close_ndi(hdd_adapter_t *adapter) { - int rc; + int errno; hdd_context_t *hdd_ctx = WLAN_HDD_GET_CTX(adapter); - uint32_t timeout = WLAN_WAIT_TIME_SESSIONOPENCLOSE; ENTER(); @@ -138,24 +137,9 @@ static int hdd_close_ndi(hdd_adapter_t *adapter) cancel_work_sync(&adapter->ipv6NotifierWorkQueue); #endif #endif - /* check if the session is open */ - if (test_bit(SME_SESSION_OPENED, &adapter->event_flags)) { - INIT_COMPLETION(adapter->session_close_comp_var); - if (QDF_STATUS_SUCCESS == sme_close_session(hdd_ctx->hHal, - adapter->sessionId, - hdd_sme_close_session_callback, adapter)) { - /* Block on a timed completion variable */ - rc = wait_for_completion_timeout( - &adapter->session_close_comp_var, - msecs_to_jiffies(timeout)); - if (!rc) - hdd_err("session close timeout"); - - rc = hdd_objmgr_release_and_destroy_vdev(adapter); - if (rc) - hdd_err("vdev delete failed"); - } - } + errno = hdd_vdev_destroy(adapter); + if (errno) + hdd_err("failed to destroy vdev: %d", errno); /* We are good to close the adapter */ hdd_close_adapter(hdd_ctx, adapter, true); diff --git a/core/hdd/src/wlan_hdd_object_manager.c b/core/hdd/src/wlan_hdd_object_manager.c index 93f559b2f243..1f662eeae41f 100644 --- a/core/hdd/src/wlan_hdd_object_manager.c +++ b/core/hdd/src/wlan_hdd_object_manager.c @@ -174,13 +174,20 @@ int hdd_objmgr_create_and_store_vdev(struct wlan_objmgr_pdev *pdev, return -ENOMEM; } + /* + * To enable legacy use cases, we need to delay physical vdev destroy + * until after the sme session has been closed. We accomplish this by + * getting an additional reference here. + */ + wlan_objmgr_vdev_get_ref(vdev, WLAN_HDD_ID_OBJ_MGR); + adapter->hdd_vdev = vdev; adapter->sessionId = wlan_vdev_get_id(vdev); return 0; } -int hdd_objmgr_release_and_destroy_vdev(hdd_adapter_t *adapter) +int hdd_objmgr_destroy_vdev(hdd_adapter_t *adapter) { struct wlan_objmgr_vdev *vdev; struct vdev_osif_priv *osif_priv; @@ -193,8 +200,6 @@ int hdd_objmgr_release_and_destroy_vdev(hdd_adapter_t *adapter) vdev->vdev_nif.osdev = NULL; qdf_mem_free(osif_priv); - adapter->hdd_vdev = NULL; - adapter->sessionId = HDD_SESSION_ID_INVALID; if (hdd_objmgr_remove_peer_object(vdev, wlan_vdev_mlme_get_macaddr(vdev))) { hdd_err("Self peer delete failed"); @@ -204,6 +209,28 @@ int hdd_objmgr_release_and_destroy_vdev(hdd_adapter_t *adapter) return qdf_status_to_os_return(wlan_objmgr_vdev_obj_delete(vdev)); } +int hdd_objmgr_release_vdev(hdd_adapter_t *adapter) +{ + /* allow physical vdev destroy by releasing the hdd reference */ + wlan_objmgr_vdev_release_ref(adapter->hdd_vdev, WLAN_HDD_ID_OBJ_MGR); + + adapter->hdd_vdev = NULL; + adapter->sessionId = HDD_SESSION_ID_INVALID; + + return 0; +} + +int hdd_objmgr_release_and_destroy_vdev(hdd_adapter_t *adapter) +{ + int errno; + + errno = hdd_objmgr_destroy_vdev(adapter); + if (errno) + return errno; + + return hdd_objmgr_release_vdev(adapter); +} + int hdd_objmgr_add_peer_object(struct wlan_objmgr_vdev *vdev, enum tQDF_ADAPTER_MODE adapter_mode, uint8_t *mac_addr) diff --git a/core/hdd/src/wlan_hdd_object_manager.h b/core/hdd/src/wlan_hdd_object_manager.h index 0fc0f38644b6..2d18c77e3329 100644 --- a/core/hdd/src/wlan_hdd_object_manager.h +++ b/core/hdd/src/wlan_hdd_object_manager.h @@ -118,9 +118,35 @@ int hdd_objmgr_release_and_destroy_pdev(hdd_context_t *hdd_ctx); int hdd_objmgr_create_and_store_vdev(struct wlan_objmgr_pdev *pdev, hdd_adapter_t *adapter); +/** + * hdd_objmgr_destroy_vdev() - Delete vdev + * @adapter: hdd adapter + * + * This function logically destroys the vdev in object manager. Physical + * deletion is prevented until the vdev is released via a call to + * hdd_objmgr_release_vdev(). E.g. + * + * hdd_objmgr_destroy_vdev(...); + * sme_close_session(...); + * hdd_objmgr_release_vdev(...); + * + * Return: 0 for success, negative error code for failure + */ +int hdd_objmgr_destroy_vdev(hdd_adapter_t *adapter); + +/** + * hdd_objmgr_release_vdev() - releases the vdev from adapter + * @adapter: hdd adapter + * + * See also hdd_objmgr_destroy_vdev() + * + * Return: 0 for success, negative error code for failure + */ +int hdd_objmgr_release_vdev(hdd_adapter_t *adapter); + /** * hdd_objmgr_release_and_destroy_vdev() - Delete vdev and remove from adapter - * @hdd_ctx: Hdd context + * @adapter: hdd adapter * * This API deletes vdev object and release its reference from hdd adapter *