From 959cb86f8258b9adb3f305c3f3d4b15d591f8f5c Mon Sep 17 00:00:00 2001 From: Zhu Jianmin Date: Thu, 26 Oct 2017 10:00:04 +0800 Subject: [PATCH] qcacmn: Fix stability caused by unwanted recursive calls in serialization Issue description: When wlan_serialization_dequeue_cmd() gets called, it will remove the command from active queue and try to move next pending command from pending queue to active queue. But before it moves this pending command to active queue completly, it calls the caller and notify that command is activated through callback. If that callback returns failure then serialization abandon the process of moving it from pending to active. Now it again picks the same command from pending and repeat the process. This creates unwanted recursion and reference count mistakenly decremented multiple times. Here is the recursion stack: (a) wlan_serialization_move_pending_to_active() (b) wlan_serialization_enqueue_cmd (c) wlan_serialization_add_cmd_to_given_queue (d) cmd_list->cmd.cmd_cb() fails (e) wlan_serialization_put_back_to_global_list (f) go back to (a) and repeat Fix: Divide wlan_serialization_enqueue_cmd() API's logic in 2 parts. 1) Enqueue the command first through wlan_serialization_enqueue_cmd() This ensures that command has been put in to given queue. 2) Once command has been put in to given queue and if this queue happen to be active queue then activate the command through wlan_serialization_activate_cmd() Here is the stack after the fix: (a) wlan_serialization_move_pending_to_active() (b) wlan_serialization_enqueue_cmd (c) wlan_serialization_add_cmd_to_given_queue (d) returns WLAN_SER_CMD_ACTIVE or !WLAN_SER_CMD_ACTIVE (a1) if (WLAN_SER_CMD_ACTIVE) (b1) wlan_serialization_put_back_to_global_list() for pending cmd (c1) wlan_serialization_activate_cmd() (d1) if fail (e1) wlan_serialization_put_back_to_global_list for active cmd (f1) then repeat from (a) (d2) if pass then return (a2) if (!WLAN_SER_CMD_ACTIVE) then nothing to be done. Change-Id: I8c63dd15d7ab8612a87950a3c189e73d1436c26a CRs-Fixed: 2132975 --- .../src/wlan_serialization_api.c | 21 +++- .../src/wlan_serialization_dequeue.c | 6 +- .../src/wlan_serialization_enqueue.c | 99 +++++++++++++------ .../src/wlan_serialization_utils_i.h | 10 ++ 4 files changed, 102 insertions(+), 34 deletions(-) diff --git a/umac/cmn_services/serialization/src/wlan_serialization_api.c b/umac/cmn_services/serialization/src/wlan_serialization_api.c index 9b7d2d02098f..20e202a526da 100644 --- a/umac/cmn_services/serialization/src/wlan_serialization_api.c +++ b/umac/cmn_services/serialization/src/wlan_serialization_api.c @@ -203,9 +203,12 @@ wlan_serialization_request(struct wlan_serialization_command *cmd) { bool is_active_cmd_allowed; QDF_STATUS status; + enum wlan_serialization_status serialization_status; uint8_t comp_id; struct wlan_serialization_psoc_priv_obj *ser_soc_obj; union wlan_serialization_rules_info info; + struct wlan_serialization_pdev_priv_obj *ser_pdev_obj = NULL; + struct wlan_objmgr_pdev *pdev = NULL; serialization_enter(); if (!cmd) { @@ -224,6 +227,18 @@ wlan_serialization_request(struct wlan_serialization_command *cmd) return WLAN_SER_CMD_DENIED_UNSPECIFIED; } + pdev = wlan_serialization_get_pdev_from_cmd(cmd); + if (!pdev) { + serialization_err("pdev is invalid"); + return WLAN_SER_CMD_DENIED_UNSPECIFIED; + } + + ser_pdev_obj = wlan_objmgr_pdev_get_comp_private_obj(pdev, + WLAN_UMAC_COMP_SERIALIZATION); + if (!ser_pdev_obj) { + serialization_err("Invalid ser_pdev_obj"); + return WLAN_SER_CMD_DENIED_UNSPECIFIED; + } /* * Get Component Info callback by calling * each registered module @@ -240,7 +255,11 @@ wlan_serialization_request(struct wlan_serialization_command *cmd) } is_active_cmd_allowed = wlan_serialization_is_active_cmd_allowed(cmd); - return wlan_serialization_enqueue_cmd(cmd, is_active_cmd_allowed); + serialization_status = wlan_serialization_enqueue_cmd( + cmd, is_active_cmd_allowed); + if (WLAN_SER_CMD_ACTIVE == serialization_status) + wlan_serialization_activate_cmd(cmd->cmd_type, ser_pdev_obj); + return serialization_status; } enum wlan_serialization_cmd_status diff --git a/umac/cmn_services/serialization/src/wlan_serialization_dequeue.c b/umac/cmn_services/serialization/src/wlan_serialization_dequeue.c index 94c361a1e2ef..e0a9e5153506 100644 --- a/umac/cmn_services/serialization/src/wlan_serialization_dequeue.c +++ b/umac/cmn_services/serialization/src/wlan_serialization_dequeue.c @@ -73,9 +73,11 @@ void wlan_serialization_move_pending_to_active( serialization_err("Can't move cmd to activeQ id-%d type-%d", cmd_list->cmd.cmd_id, cmd_list->cmd.cmd_type); return; - } - wlan_serialization_put_back_to_global_list(pending_queue, + } else { + wlan_serialization_put_back_to_global_list(pending_queue, ser_pdev_obj, cmd_list); + wlan_serialization_activate_cmd(cmd_type, ser_pdev_obj); + } return; } diff --git a/umac/cmn_services/serialization/src/wlan_serialization_enqueue.c b/umac/cmn_services/serialization/src/wlan_serialization_enqueue.c index 1cff45d25bb9..fcc1725fdc10 100644 --- a/umac/cmn_services/serialization/src/wlan_serialization_enqueue.c +++ b/umac/cmn_services/serialization/src/wlan_serialization_enqueue.c @@ -77,43 +77,80 @@ wlan_serialization_add_cmd_to_given_queue(qdf_list_t *queue, return WLAN_SER_CMD_DENIED_UNSPECIFIED; } - if (is_cmd_for_active_queue) { - /* - * command is already pushed to active queue above - * now start the timer and notify requestor - */ - wlan_serialization_find_and_start_timer(psoc, - &cmd_list->cmd); - if (cmd_list->cmd.cmd_cb) { - /* - * Remember that serialization module may send - * this callback in same context through which it - * received the serialization request. Due to which - * it is caller's responsibility to ensure acquiring - * and releasing its own lock appropriately. - */ - qdf_status = cmd_list->cmd.cmd_cb(&cmd_list->cmd, - WLAN_SER_CB_ACTIVATE_CMD); - if (qdf_status != QDF_STATUS_SUCCESS) { - wlan_serialization_find_and_stop_timer(psoc, - &cmd_list->cmd); - cmd_list->cmd.cmd_cb(&cmd_list->cmd, - WLAN_SER_CB_RELEASE_MEM_CMD); - wlan_serialization_put_back_to_global_list( - queue, ser_pdev_obj, cmd_list); - wlan_serialization_move_pending_to_active( - cmd_list->cmd.cmd_type, - ser_pdev_obj); - } - } + if (is_cmd_for_active_queue) status = WLAN_SER_CMD_ACTIVE; - } else { + else status = WLAN_SER_CMD_PENDING; - } return status; } +void wlan_serialization_activate_cmd(enum wlan_serialization_cmd_type cmd_type, + struct wlan_serialization_pdev_priv_obj *ser_pdev_obj) +{ + QDF_STATUS qdf_status = QDF_STATUS_SUCCESS; + qdf_list_t *queue = NULL; + qdf_list_node_t *nnode = NULL; + struct wlan_serialization_command_list *cmd_list = NULL; + struct wlan_objmgr_psoc *psoc = NULL; + + if (cmd_type < WLAN_SER_CMD_NONSCAN) + queue = &ser_pdev_obj->active_scan_list; + else + queue = &ser_pdev_obj->active_list; + if (qdf_list_empty(queue)) { + serialization_err("nothing in active queue"); + QDF_ASSERT(0); + return; + } + if (QDF_STATUS_SUCCESS != qdf_list_peek_front(queue, &nnode)) { + serialization_err("can't read from active queue"); + serialization_debug("cmd_type - %d", cmd_type); + return; + } + cmd_list = qdf_container_of(nnode, + struct wlan_serialization_command_list, node); + + /* + * command is already pushed to active queue above + * now start the timer and notify requestor + */ + wlan_serialization_find_and_start_timer(psoc, + &cmd_list->cmd); + if (cmd_list && cmd_list->cmd.cmd_cb) { + if (cmd_list->cmd.vdev) { + psoc = wlan_vdev_get_psoc(cmd_list->cmd.vdev); + if (psoc == NULL) { + serialization_err("invalid psoc"); + return; + } + } else { + serialization_err("invalid cmd.vdev"); + return; + } + /* + * Remember that serialization module may send + * this callback in same context through which it + * received the serialization request. Due to which + * it is caller's responsibility to ensure acquiring + * and releasing its own lock appropriately. + */ + qdf_status = cmd_list->cmd.cmd_cb(&cmd_list->cmd, + WLAN_SER_CB_ACTIVATE_CMD); + if (qdf_status != QDF_STATUS_SUCCESS) { + wlan_serialization_find_and_stop_timer(psoc, + &cmd_list->cmd); + cmd_list->cmd.cmd_cb(&cmd_list->cmd, + WLAN_SER_CB_RELEASE_MEM_CMD); + wlan_serialization_put_back_to_global_list( + queue, ser_pdev_obj, cmd_list); + wlan_serialization_move_pending_to_active( + cmd_list->cmd.cmd_type, + ser_pdev_obj); + } + } +} + enum wlan_serialization_status wlan_serialization_enqueue_cmd(struct wlan_serialization_command *cmd, uint8_t is_cmd_for_active_queue) diff --git a/umac/cmn_services/serialization/src/wlan_serialization_utils_i.h b/umac/cmn_services/serialization/src/wlan_serialization_utils_i.h index 7dad59512609..c0fdf2b6a128 100644 --- a/umac/cmn_services/serialization/src/wlan_serialization_utils_i.h +++ b/umac/cmn_services/serialization/src/wlan_serialization_utils_i.h @@ -398,4 +398,14 @@ wlan_serialization_remove_all_cmd_from_queue(qdf_list_t *queue, bool wlan_serialization_is_cmd_present_queue( struct wlan_serialization_command *cmd, uint8_t is_active_queue); + +/** + * wlan_serialization_activate_cmd() - activate first cmd in active queue + * @cmd_type: Command Type + * @ser_pdev_obj: Serialization private pdev object + * + * Return: None + */ +void wlan_serialization_activate_cmd(enum wlan_serialization_cmd_type cmd_type, + struct wlan_serialization_pdev_priv_obj *ser_pdev_obj); #endif