From 583fb300c8fe93f842ef01c35eed72175b3f9d8d Mon Sep 17 00:00:00 2001 From: Nirmal Abraham Date: Thu, 3 Feb 2022 12:09:19 +0530 Subject: [PATCH 1/3] msm: camera: reqmgr: Avoid freeing subdev twice The 'l_device' pointer in __cam_req_mgr_destroy_subdev is set to NULL after freeing but this is done on a local copy of the variable in stack. This results in double-free when this function is called again. To avoid this, pass 'l_device' pointer by reference and assign it to NULL after freeing. CRs-Fixed: 3120468 Change-Id: If2dde6f1c702bee26a3c8a68c2f45bafbf0f7cd6 Signed-off-by: Nirmal Abraham --- drivers/cam_req_mgr/cam_req_mgr_core.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/cam_req_mgr/cam_req_mgr_core.c b/drivers/cam_req_mgr/cam_req_mgr_core.c index 18dcb1bc2b96..80eed1cfd526 100644 --- a/drivers/cam_req_mgr/cam_req_mgr_core.c +++ b/drivers/cam_req_mgr/cam_req_mgr_core.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2016-2021, The Linux Foundation. All rights reserved. + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -2281,10 +2282,13 @@ static int __cam_req_mgr_create_subdevs( * */ static void __cam_req_mgr_destroy_subdev( - struct cam_req_mgr_connected_device *l_device) + struct cam_req_mgr_connected_device **l_device) { - kfree(l_device); - l_device = NULL; + CAM_DBG(CAM_CRM, "*l_device %pK", *l_device); + if (*(l_device) != NULL) { + kfree(*(l_device)); + *l_device = NULL; + } } /** @@ -3820,7 +3824,7 @@ static int __cam_req_mgr_unlink(struct cam_req_mgr_core_link *link) __cam_req_mgr_destroy_link_info(link); /* Free memory holding data of linked devs */ - __cam_req_mgr_destroy_subdev(link->l_dev); + __cam_req_mgr_destroy_subdev(&link->l_dev); /* Destroy the link handle */ rc = cam_destroy_device_hdl(link->link_hdl); @@ -3995,7 +3999,7 @@ int cam_req_mgr_link(struct cam_req_mgr_ver_info *link_info) mutex_unlock(&g_crm_core_dev->crm_lock); return rc; setup_failed: - __cam_req_mgr_destroy_subdev(link->l_dev); + __cam_req_mgr_destroy_subdev(&link->l_dev); create_subdev_failed: cam_destroy_device_hdl(link->link_hdl); link_info->u.link_info_v1.link_hdl = -1; @@ -4111,7 +4115,7 @@ int cam_req_mgr_link_v2(struct cam_req_mgr_ver_info *link_info) mutex_unlock(&g_crm_core_dev->crm_lock); return rc; setup_failed: - __cam_req_mgr_destroy_subdev(link->l_dev); + __cam_req_mgr_destroy_subdev(&link->l_dev); create_subdev_failed: cam_destroy_device_hdl(link->link_hdl); link_info->u.link_info_v2.link_hdl = -1; From f33380f1828c21262133e60112422c7493498f26 Mon Sep 17 00:00:00 2001 From: zhuo Date: Tue, 4 Jan 2022 10:54:30 +0800 Subject: [PATCH 2/3] msm: camera: cdm: Fix workqueue timing issue Due to workqueue does not process the work in order, so sometimes the later work will be processed earlier. Such as, when submit request order: 1/2/3, cdm interrupt come order: 1/2/3, workqueue process order: 2/1/3, when process 2 request, which currently will notify 1/2 CDM clients and remove 1/2 from submit list. After that, when process 1 request, will notify 3, actually 3 is not done at the moment, which maybe cause smmu page fault issue. And sometimes, when there is a delay in handling interrupts, then HLOS handles two interrupts as one only. This change only notify the request less than and equal to the interrupt request. CRs-Fixed: 3130447 Change-Id: I0fd0e8adee48767e5ab7db1921a8284d107c2f40 Signed-off-by: zhuo --- drivers/cam_cdm/cam_cdm.h | 3 +++ drivers/cam_cdm/cam_cdm_hw_core.c | 39 +++++++++++++++++-------------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/drivers/cam_cdm/cam_cdm.h b/drivers/cam_cdm/cam_cdm.h index dc2c164e0b2c..df19116abab6 100644 --- a/drivers/cam_cdm/cam_cdm.h +++ b/drivers/cam_cdm/cam_cdm.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. */ #ifndef _CAM_CDM_H_ @@ -68,6 +69,8 @@ #define CAM_CDM_BL_FIFO_LENGTH_MAX_DEFAULT 0x40 #define CAM_CDM_BL_FIFO_LENGTH_CFG_SHIFT 0x10 #define CAM_CDM_BL_FIFO_FLUSH_SHIFT 0x3 +#define CAM_CDM_BL_FIFO_BOUNDARY_CHECK \ + (CAM_CDM_BL_FIFO_LENGTH_MAX_DEFAULT / 2) #define CAM_CDM_BL_FIFO_REQ_SIZE_MAX 0x00 #define CAM_CDM_BL_FIFO_REQ_SIZE_MAX_DIV2 0x01 diff --git a/drivers/cam_cdm/cam_cdm_hw_core.c b/drivers/cam_cdm/cam_cdm_hw_core.c index bd1e938a0d0f..901f84c3e034 100644 --- a/drivers/cam_cdm/cam_cdm_hw_core.c +++ b/drivers/cam_cdm/cam_cdm_hw_core.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2021, The Linux Foundation. All rights reserved. + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -1263,26 +1264,30 @@ static void cam_hw_cdm_work(struct work_struct *work) list_for_each_entry_safe(node, tnode, &core->bl_fifo[fifo_idx].bl_request_list, entry) { - if (node->request_type == - CAM_HW_CDM_BL_CB_CLIENT) { - cam_cdm_notify_clients(cdm_hw, - CAM_CDM_CB_STATUS_BL_SUCCESS, - (void *)node); - } else if (node->request_type == - CAM_HW_CDM_BL_CB_INTERNAL) { - CAM_ERR(CAM_CDM, - "Invalid node=%pK %d", - node, - node->request_type); - } - list_del_init(&node->entry); - if (node->bl_tag == payload->irq_data) { + if ((node->bl_tag <= payload->irq_data) || + ((node->bl_tag - payload->irq_data) > + CAM_CDM_BL_FIFO_BOUNDARY_CHECK)) { + if (node->request_type == + CAM_HW_CDM_BL_CB_CLIENT) { + cam_cdm_notify_clients(cdm_hw, + CAM_CDM_CB_STATUS_BL_SUCCESS, + (void *)node); + } else if (node->request_type == + CAM_HW_CDM_BL_CB_INTERNAL) { + CAM_ERR(CAM_CDM, + "Invalid node=%pK %d", + node, + node->request_type); + } + list_del_init(&node->entry); + if (node->bl_tag == payload->irq_data) { + kfree(node); + node = NULL; + break; + } kfree(node); node = NULL; - break; } - kfree(node); - node = NULL; } } else { CAM_INFO(CAM_CDM, From e5cbfaf0af5149271e57deca9f47b5a98ef407ac Mon Sep 17 00:00:00 2001 From: Yash Upadhyay Date: Mon, 21 Feb 2022 10:07:23 +0530 Subject: [PATCH 3/3] msm: camera: reqmgr: Validate the link handle Instead of correct link handle, if some other handle like dev handle is passed then it may access some other data space. To avoid such scenario, need to check whether link handle passed by ioctl is same as retrieved link handle. CRs-Fixed: 3120454 Change-Id: Idff2e3c25b60563788ffb426c7cabc367c3c97f8 Signed-off-by: Yash Upadhyay --- drivers/cam_req_mgr/cam_req_mgr_core.c | 80 +++++++++++++++++--------- drivers/cam_req_mgr/cam_req_mgr_core.h | 2 + drivers/cam_utils/cam_debug_util.h | 3 + 3 files changed, 57 insertions(+), 28 deletions(-) diff --git a/drivers/cam_req_mgr/cam_req_mgr_core.c b/drivers/cam_req_mgr/cam_req_mgr_core.c index ac04553899a2..5d37b3edc99a 100644 --- a/drivers/cam_req_mgr/cam_req_mgr_core.c +++ b/drivers/cam_req_mgr/cam_req_mgr_core.c @@ -3857,8 +3857,10 @@ int cam_req_mgr_destroy_session( mutex_lock(&g_crm_core_dev->crm_lock); cam_session = (struct cam_req_mgr_core_session *) cam_get_device_priv(ses_info->session_hdl); - if (!cam_session) { - CAM_ERR(CAM_CRM, "failed to get session priv"); + if (!cam_session || (cam_session->session_hdl != ses_info->session_hdl)) { + CAM_ERR(CAM_CRM, "session: %s, ses_info->ses_hdl:%x, session->ses_hdl:%x", + CAM_IS_NULL_TO_STR(cam_session), ses_info->session_hdl, + (!cam_session) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : cam_session->session_hdl); rc = -ENOENT; goto end; @@ -3928,8 +3930,10 @@ int cam_req_mgr_link(struct cam_req_mgr_ver_info *link_info) /* session hdl's priv data is cam session struct */ cam_session = (struct cam_req_mgr_core_session *) cam_get_device_priv(link_info->u.link_info_v1.session_hdl); - if (!cam_session) { - CAM_DBG(CAM_CRM, "NULL pointer"); + if (!cam_session || (cam_session->session_hdl != link_info->u.link_info_v1.session_hdl)) { + CAM_ERR(CAM_CRM, "session: %s, link_info->ses_hdl:%x, session->ses_hdl:%x", + CAM_IS_NULL_TO_STR(cam_session), link_info->u.link_info_v1.session_hdl, + (!cam_session) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : cam_session->session_hdl); mutex_unlock(&g_crm_core_dev->crm_lock); return -EINVAL; } @@ -4038,8 +4042,10 @@ int cam_req_mgr_link_v2(struct cam_req_mgr_ver_info *link_info) /* session hdl's priv data is cam session struct */ cam_session = (struct cam_req_mgr_core_session *) cam_get_device_priv(link_info->u.link_info_v2.session_hdl); - if (!cam_session) { - CAM_DBG(CAM_CRM, "NULL pointer"); + if (!cam_session || (cam_session->session_hdl != link_info->u.link_info_v2.session_hdl)) { + CAM_ERR(CAM_CRM, "session: %s, link_info->ses_hdl:%x, session->ses_hdl:%x", + CAM_IS_NULL_TO_STR(cam_session), link_info->u.link_info_v2.session_hdl, + (!cam_session) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : cam_session->session_hdl); mutex_unlock(&g_crm_core_dev->crm_lock); return -EINVAL; } @@ -4147,16 +4153,20 @@ int cam_req_mgr_unlink(struct cam_req_mgr_unlink_info *unlink_info) /* session hdl's priv data is cam session struct */ cam_session = (struct cam_req_mgr_core_session *) cam_get_device_priv(unlink_info->session_hdl); - if (!cam_session) { - CAM_ERR(CAM_CRM, "NULL pointer"); + if (!cam_session || (cam_session->session_hdl != unlink_info->session_hdl)) { + CAM_ERR(CAM_CRM, "session: %s, unlink_info->ses_hdl:%x, cam_session->ses_hdl:%x", + CAM_IS_NULL_TO_STR(cam_session), unlink_info->session_hdl, + (!cam_session) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : cam_session->session_hdl); mutex_unlock(&g_crm_core_dev->crm_lock); return -EINVAL; } /* link hdl's priv data is core_link struct */ link = cam_get_device_priv(unlink_info->link_hdl); - if (!link) { - CAM_ERR(CAM_CRM, "NULL pointer"); + if (!link || (link->link_hdl != unlink_info->link_hdl)) { + CAM_ERR(CAM_CRM, "link: %s, unlink_info->link_hdl:%x, link->link_hdl:%x", + CAM_IS_NULL_TO_STR(link), unlink_info->link_hdl, + (!link) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : link->link_hdl); rc = -EINVAL; goto done; } @@ -4189,8 +4199,10 @@ int cam_req_mgr_schedule_request( mutex_lock(&g_crm_core_dev->crm_lock); link = (struct cam_req_mgr_core_link *) cam_get_device_priv(sched_req->link_hdl); - if (!link) { - CAM_DBG(CAM_CRM, "link ptr NULL %x", sched_req->link_hdl); + if (!link || (link->link_hdl != sched_req->link_hdl)) { + CAM_ERR(CAM_CRM, "link: %s, sched_req->link_hdl:%x, link->link_hdl:%x", + CAM_IS_NULL_TO_STR(link), sched_req->link_hdl, + (!link) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : link->link_hdl); rc = -EINVAL; goto end; } @@ -4274,8 +4286,10 @@ int cam_req_mgr_sync_config( /* session hdl's priv data is cam session struct */ cam_session = (struct cam_req_mgr_core_session *) cam_get_device_priv(sync_info->session_hdl); - if (!cam_session) { - CAM_ERR(CAM_CRM, "NULL pointer"); + if (!cam_session || (cam_session->session_hdl != sync_info->session_hdl)) { + CAM_ERR(CAM_CRM, "session: %s, sync_info->session_hdl:%x, session->ses_hdl:%x", + CAM_IS_NULL_TO_STR(cam_session), sync_info->session_hdl, + (!cam_session) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : cam_session->session_hdl); mutex_unlock(&g_crm_core_dev->crm_lock); return -EINVAL; } @@ -4291,8 +4305,10 @@ int cam_req_mgr_sync_config( } link[i] = cam_get_device_priv(sync_info->link_hdls[i]); - if (!link[i]) { - CAM_ERR(CAM_CRM, "link%d NULL pointer", i); + if (!link[i] || (link[i]->link_hdl != sync_info->link_hdls[i])) { + CAM_ERR(CAM_CRM, "link: %s, sync_info->link_hdl:%x, link->link_hdl:%x", + CAM_IS_NULL_TO_STR(link), sync_info->link_hdls[i], + (!link[i]) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : link[i]->link_hdl); rc = -EINVAL; goto done; } @@ -4368,8 +4384,10 @@ int cam_req_mgr_flush_requests( /* session hdl's priv data is cam session struct */ session = (struct cam_req_mgr_core_session *) cam_get_device_priv(flush_info->session_hdl); - if (!session) { - CAM_ERR(CAM_CRM, "Invalid session %x", flush_info->session_hdl); + if (!session || (session->session_hdl != flush_info->session_hdl)) { + CAM_ERR(CAM_CRM, "session: %s, flush_info->ses_hdl:%x, session->ses_hdl:%x", + CAM_IS_NULL_TO_STR(session), flush_info->session_hdl, + (!session) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : session->session_hdl); rc = -EINVAL; goto end; } @@ -4381,8 +4399,10 @@ int cam_req_mgr_flush_requests( link = (struct cam_req_mgr_core_link *) cam_get_device_priv(flush_info->link_hdl); - if (!link) { - CAM_DBG(CAM_CRM, "link ptr NULL %x", flush_info->link_hdl); + if (!link || (link->link_hdl != flush_info->link_hdl)) { + CAM_ERR(CAM_CRM, "link: %s, flush_info->link_hdl:%x, link->link_hdl:%x", + CAM_IS_NULL_TO_STR(link), flush_info->link_hdl, + (!link) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : link->link_hdl); rc = -EINVAL; goto end; } @@ -4440,9 +4460,10 @@ int cam_req_mgr_link_control(struct cam_req_mgr_link_control *control) for (i = 0; i < control->num_links; i++) { link = (struct cam_req_mgr_core_link *) cam_get_device_priv(control->link_hdls[i]); - if (!link) { - CAM_ERR(CAM_CRM, "Link(%d) is NULL on session 0x%x", - i, control->session_hdl); + if (!link || (link->link_hdl != control->link_hdls[i])) { + CAM_ERR(CAM_CRM, "link: %s, control->link_hdl:%x, link->link_hdl:%x", + CAM_IS_NULL_TO_STR(link), control->link_hdls[i], + (!link) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : link->link_hdl); rc = -EINVAL; break; } @@ -4528,9 +4549,10 @@ int cam_req_mgr_dump_request(struct cam_dump_req_cmd *dump_req) /* session hdl's priv data is cam session struct */ session = (struct cam_req_mgr_core_session *) cam_get_device_priv(dump_req->session_handle); - if (!session) { - CAM_ERR(CAM_CRM, "Invalid session %x", - dump_req->session_handle); + if (!session || (session->session_hdl != dump_req->session_handle)) { + CAM_ERR(CAM_CRM, "session: %s, dump_req->ses_hdl:%x, session->ses_hdl:%x", + CAM_IS_NULL_TO_STR(session), dump_req->session_handle, + (!session) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : session->session_hdl); rc = -EINVAL; goto end; } @@ -4542,8 +4564,10 @@ int cam_req_mgr_dump_request(struct cam_dump_req_cmd *dump_req) link = (struct cam_req_mgr_core_link *) cam_get_device_priv(dump_req->link_hdl); - if (!link || link->link_hdl != dump_req->link_hdl) { - CAM_DBG(CAM_CRM, "link ptr NULL %x", dump_req->link_hdl); + if (!link || (link->link_hdl != dump_req->link_hdl)) { + CAM_ERR(CAM_CRM, "link: %s, dump_req->link_hdl:%x, link->link_hdl:%x", + CAM_IS_NULL_TO_STR(link), dump_req->link_hdl, + (!link) ? CAM_REQ_MGR_DEFAULT_HDL_VAL : link->link_hdl); rc = -EINVAL; goto end; } diff --git a/drivers/cam_req_mgr/cam_req_mgr_core.h b/drivers/cam_req_mgr/cam_req_mgr_core.h index 8b6edaa3f9ff..6f75677c29c2 100644 --- a/drivers/cam_req_mgr/cam_req_mgr_core.h +++ b/drivers/cam_req_mgr/cam_req_mgr_core.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* * Copyright (c) 2016-2021, The Linux Foundation. All rights reserved. + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. */ #ifndef _CAM_REQ_MGR_CORE_H_ #define _CAM_REQ_MGR_CORE_H_ @@ -18,6 +19,7 @@ #define CAM_REQ_MGR_WATCHDOG_TIMEOUT_MAX 50000 #define CAM_REQ_MGR_SCHED_REQ_TIMEOUT 1000 #define CAM_REQ_MGR_SIMULATE_SCHED_REQ 30 +#define CAM_REQ_MGR_DEFAULT_HDL_VAL 0 #define FORCE_DISABLE_RECOVERY 2 #define FORCE_ENABLE_RECOVERY 1 diff --git a/drivers/cam_utils/cam_debug_util.h b/drivers/cam_utils/cam_debug_util.h index 5232501525d1..28fc4c2399a2 100644 --- a/drivers/cam_utils/cam_debug_util.h +++ b/drivers/cam_utils/cam_debug_util.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. */ #ifndef _CAM_DEBUG_UTIL_H_ @@ -8,6 +9,8 @@ #include +#define CAM_IS_NULL_TO_STR(ptr) ((ptr) ? "Non-NULL" : "NULL") + /* Module IDs used for debug logging */ #define CAM_CDM (1 << 0) #define CAM_CORE (1 << 1)