From 0c9079d6e20382b5f0fa5a1b8598c7e4521730be Mon Sep 17 00:00:00 2001 From: Yash Upadhyay Date: Fri, 16 Feb 2024 09:14:07 +0530 Subject: [PATCH 1/4] msm: camera: common: Fix possible OOB reads and writes operations We need to check if the packet is valid before using it. CRs-Fixed: 3605421 Change-Id: Ide4e005ba46690c1cac02cb77a2d9aaa497b15df Signed-off-by: Yash Upadhyay --- drivers/cam_fd/fd_hw_mgr/cam_fd_hw_mgr.c | 10 +++++- .../icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c | 10 +++++- drivers/cam_isp/isp_hw_mgr/cam_ife_hw_mgr.c | 6 +++- .../hw_utils/cam_isp_packet_parser.c | 6 +++- .../cam_lrme/lrme_hw_mgr/cam_lrme_hw_mgr.c | 10 +++++- drivers/cam_ope/ope_hw_mgr/cam_ope_hw_mgr.c | 34 ++++++++++++++----- .../cam_actuator/cam_actuator_core.c | 6 +++- .../cam_eeprom/cam_eeprom_core.c | 10 +++++- .../cam_flash/cam_flash_core.c | 6 +++- .../cam_sensor_module/cam_ois/cam_ois_core.c | 6 +++- .../cam_sensor/cam_sensor_core.c | 5 +++ 11 files changed, 91 insertions(+), 18 deletions(-) diff --git a/drivers/cam_fd/fd_hw_mgr/cam_fd_hw_mgr.c b/drivers/cam_fd/fd_hw_mgr/cam_fd_hw_mgr.c index d3110ba5ae89..67670c480141 100644 --- a/drivers/cam_fd/fd_hw_mgr/cam_fd_hw_mgr.c +++ b/drivers/cam_fd/fd_hw_mgr/cam_fd_hw_mgr.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -78,6 +78,10 @@ static int cam_fd_mgr_util_packet_validate(struct cam_packet *packet, packet->cmd_buf_offset); for (i = 0; i < packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + /* * We can allow 0 length cmd buffer. This can happen in case * umd gives an empty cmd buffer as kmd buffer @@ -789,6 +793,10 @@ static int cam_fd_mgr_util_prepare_hw_update_entries( &prepare->packet->payload + prepare->packet->cmd_buf_offset); for (i = 0; i < prepare->packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + if (!cmd_desc[i].length) continue; diff --git a/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c b/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c index fc3f73be380a..73778c11658c 100644 --- a/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c +++ b/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -4596,6 +4596,10 @@ static int cam_icp_process_generic_cmd_buffer( cmd_desc = (struct cam_cmd_buf_desc *) ((uint32_t *) &packet->payload + packet->cmd_buf_offset/4); for (i = 0; i < packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + if (!cmd_desc[i].length) continue; @@ -4781,6 +4785,10 @@ static int cam_icp_mgr_config_stream_settings( cmd_desc = (struct cam_cmd_buf_desc *) ((uint32_t *) &packet->payload + packet->cmd_buf_offset/4); + rc = cam_packet_util_validate_cmd_desc(cmd_desc); + if (rc) + return rc; + if (!cmd_desc[0].length || cmd_desc[0].meta_data != CAM_ICP_CMD_META_GENERIC_BLOB) { CAM_ERR(CAM_ICP, "Invalid cmd buffer length/metadata"); diff --git a/drivers/cam_isp/isp_hw_mgr/cam_ife_hw_mgr.c b/drivers/cam_isp/isp_hw_mgr/cam_ife_hw_mgr.c index e61591714bbe..a9a8416c6bd8 100644 --- a/drivers/cam_isp/isp_hw_mgr/cam_ife_hw_mgr.c +++ b/drivers/cam_isp/isp_hw_mgr/cam_ife_hw_mgr.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -154,6 +154,10 @@ static int cam_ife_mgr_handle_reg_dump(struct cam_ife_hw_mgr_ctx *ctx, "Reg dump values might be from more than one request"); for (i = 0; i < num_reg_dump_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(®_dump_buf_desc[i]); + if (rc) + return rc; + CAM_DBG(CAM_ISP, "Reg dump cmd meta data: %u req_type: %u", reg_dump_buf_desc[i].meta_data, meta_type); if (reg_dump_buf_desc[i].meta_data == meta_type) { diff --git a/drivers/cam_isp/isp_hw_mgr/hw_utils/cam_isp_packet_parser.c b/drivers/cam_isp/isp_hw_mgr/hw_utils/cam_isp_packet_parser.c index 7c43f331a009..8298e2126f88 100644 --- a/drivers/cam_isp/isp_hw_mgr/hw_utils/cam_isp_packet_parser.c +++ b/drivers/cam_isp/isp_hw_mgr/hw_utils/cam_isp_packet_parser.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2021, The Linux Foundation. All rights reserved. - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -283,6 +283,10 @@ int cam_isp_add_command_buffers( split_id, prepare->packet->num_cmd_buf); for (i = 0; i < prepare->packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + num_ent = prepare->num_hw_update_entries; if (!cmd_desc[i].length) continue; diff --git a/drivers/cam_lrme/lrme_hw_mgr/cam_lrme_hw_mgr.c b/drivers/cam_lrme/lrme_hw_mgr/cam_lrme_hw_mgr.c index 3d1857f05b63..ecf56ad6eb21 100644 --- a/drivers/cam_lrme/lrme_hw_mgr/cam_lrme_hw_mgr.c +++ b/drivers/cam_lrme/lrme_hw_mgr/cam_lrme_hw_mgr.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -117,6 +117,10 @@ static int cam_lrme_mgr_util_packet_validate(struct cam_packet *packet, packet->cmd_buf_offset); for (i = 0; i < packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + if (!cmd_desc[i].length) continue; @@ -317,6 +321,10 @@ static int cam_lrme_mgr_util_prepare_hw_update_entries( &prepare->packet->payload + prepare->packet->cmd_buf_offset); for (i = 0; i < prepare->packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + if (!cmd_desc[i].length) continue; diff --git a/drivers/cam_ope/ope_hw_mgr/cam_ope_hw_mgr.c b/drivers/cam_ope/ope_hw_mgr/cam_ope_hw_mgr.c index 1753d8f0e75e..e5aae0120447 100644 --- a/drivers/cam_ope/ope_hw_mgr/cam_ope_hw_mgr.c +++ b/drivers/cam_ope/ope_hw_mgr/cam_ope_hw_mgr.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2021, The Linux Foundation. All rights reserved. - * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -427,13 +427,17 @@ static void cam_ope_dump_dmi(struct cam_ope_hang_dump *dump, uint32_t addr, static int cam_ope_mgr_put_cmd_buf(struct cam_packet *packet) { - int i = 0; + int i = 0, rc = 0; struct cam_cmd_buf_desc *cmd_desc = NULL; cmd_desc = (struct cam_cmd_buf_desc *) ((uint32_t *) &packet->payload + packet->cmd_buf_offset/4); for (i = 0; i < packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + if (cmd_desc[i].type != CAM_CMD_BUF_GENERIC || cmd_desc[i].meta_data == OPE_CMD_META_GENERIC_BLOB) continue; @@ -441,7 +445,7 @@ static int cam_ope_mgr_put_cmd_buf(struct cam_packet *packet) cam_mem_put_cpu_buf(cmd_desc[i].mem_handle); } - return 0; + return rc; } static int cam_ope_dump_indirect(struct ope_cmd_buf_info *cmd_buf_info, @@ -555,6 +559,10 @@ static int cam_ope_dump_frame_process(struct cam_packet *packet, cmd_desc = (struct cam_cmd_buf_desc *) ((uint32_t *) &packet->payload + packet->cmd_buf_offset/4); for (i = 0; i < packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + if (cmd_desc[i].type != CAM_CMD_BUF_GENERIC || cmd_desc[i].meta_data == OPE_CMD_META_GENERIC_BLOB) continue; @@ -2276,6 +2284,10 @@ static int cam_ope_mgr_process_cmd_desc(struct cam_ope_hw_mgr *hw_mgr, *ope_cmd_buf_addr = 0; for (i = 0; i < packet->num_cmd_buf; i++, num_cmd_buf++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + if (cmd_desc[i].type != CAM_CMD_BUF_GENERIC || cmd_desc[i].meta_data == OPE_CMD_META_GENERIC_BLOB) continue; @@ -3167,16 +3179,20 @@ static int cam_ope_process_generic_cmd_buffer( ((uint32_t *) &packet->payload + packet->cmd_buf_offset/4); for (i = 0; i < packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + if (!cmd_desc[i].length) continue; - if (cmd_desc[i].meta_data != OPE_CMD_META_GENERIC_BLOB) - continue; + if (cmd_desc[i].meta_data != OPE_CMD_META_GENERIC_BLOB) + continue; - rc = cam_packet_util_process_generic_cmd_buffer(&cmd_desc[i], - cam_ope_packet_generic_blob_handler, &cmd_generic_blob); - if (rc) - CAM_ERR(CAM_OPE, "Failed in processing blobs %d", rc); + rc = cam_packet_util_process_generic_cmd_buffer(&cmd_desc[i], + cam_ope_packet_generic_blob_handler, &cmd_generic_blob); + if (rc) + CAM_ERR(CAM_OPE, "Failed in processing blobs %d", rc); } return rc; diff --git a/drivers/cam_sensor_module/cam_actuator/cam_actuator_core.c b/drivers/cam_sensor_module/cam_actuator/cam_actuator_core.c index 8159ea75a1eb..46765e5b2e2b 100644 --- a/drivers/cam_sensor_module/cam_actuator/cam_actuator_core.c +++ b/drivers/cam_sensor_module/cam_actuator/cam_actuator_core.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -489,6 +489,10 @@ int32_t cam_actuator_i2c_pkt_parse(struct cam_actuator_ctrl_t *a_ctrl, /* Loop through multiple command buffers */ for (i = 0; i < csl_packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + total_cmd_buf_in_bytes = cmd_desc[i].length; if (!total_cmd_buf_in_bytes) continue; diff --git a/drivers/cam_sensor_module/cam_eeprom/cam_eeprom_core.c b/drivers/cam_sensor_module/cam_eeprom/cam_eeprom_core.c index 3e5b3c45ead2..a25611df5061 100644 --- a/drivers/cam_sensor_module/cam_eeprom/cam_eeprom_core.c +++ b/drivers/cam_sensor_module/cam_eeprom/cam_eeprom_core.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. - * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -729,6 +729,10 @@ static int32_t cam_eeprom_parse_write_memory_packet( int master; struct cam_sensor_cci_client *cci; + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + total_cmd_buf_in_bytes = cmd_desc[i].length; processed_cmd_buf_in_bytes = 0; @@ -946,6 +950,10 @@ static int32_t cam_eeprom_init_pkt_parser(struct cam_eeprom_ctrl_t *e_ctrl, /* Loop through multiple command buffers */ for (i = 0; i < csl_packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + total_cmd_buf_in_bytes = cmd_desc[i].length; processed_cmd_buf_in_bytes = 0; if (!total_cmd_buf_in_bytes) diff --git a/drivers/cam_sensor_module/cam_flash/cam_flash_core.c b/drivers/cam_sensor_module/cam_flash/cam_flash_core.c index 9007e5642fdd..f74987d9b112 100644 --- a/drivers/cam_sensor_module/cam_flash/cam_flash_core.c +++ b/drivers/cam_sensor_module/cam_flash/cam_flash_core.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2021, The Linux Foundation. All rights reserved. - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -1076,6 +1076,10 @@ int cam_flash_i2c_pkt_parser(struct cam_flash_ctrl *fctrl, void *arg) /* Loop through multiple command buffers */ for (i = 1; i < csl_packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + total_cmd_buf_in_bytes = cmd_desc[i].length; processed_cmd_buf_in_bytes = 0; if (!total_cmd_buf_in_bytes) diff --git a/drivers/cam_sensor_module/cam_ois/cam_ois_core.c b/drivers/cam_sensor_module/cam_ois/cam_ois_core.c index 16c93f74624b..aa60bddfc073 100644 --- a/drivers/cam_sensor_module/cam_ois/cam_ois_core.c +++ b/drivers/cam_sensor_module/cam_ois/cam_ois_core.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -483,6 +483,10 @@ static int cam_ois_pkt_parse(struct cam_ois_ctrl_t *o_ctrl, void *arg) /* Loop through multiple command buffers */ for (i = 0; i < csl_packet->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + total_cmd_buf_in_bytes = cmd_desc[i].length; if (!total_cmd_buf_in_bytes) continue; diff --git a/drivers/cam_sensor_module/cam_sensor/cam_sensor_core.c b/drivers/cam_sensor_module/cam_sensor/cam_sensor_core.c index 8f454f1a24f8..37d3ea2fa1d7 100644 --- a/drivers/cam_sensor_module/cam_sensor/cam_sensor_core.c +++ b/drivers/cam_sensor_module/cam_sensor/cam_sensor_core.c @@ -2,6 +2,7 @@ /* * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. * Copyright (c) 2023, The Linux Foundation. All rights reserved. + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -494,6 +495,10 @@ int32_t cam_handle_mem_ptr(uint64_t handle, struct cam_sensor_ctrl_t *s_ctrl) } for (i = 0; i < pkt->num_cmd_buf; i++) { + rc = cam_packet_util_validate_cmd_desc(&cmd_desc[i]); + if (rc) + return rc; + if (!(cmd_desc[i].length)) continue; rc = cam_mem_get_cpu_buf(cmd_desc[i].mem_handle, From 97e98e4974a045986e0bce3cf41db6e03394315f Mon Sep 17 00:00:00 2001 From: Shivi Mangal Date: Thu, 21 Mar 2024 17:36:40 -0700 Subject: [PATCH 2/4] msm: camera: sensor: Handling race condition in util api I2C cmd is coming from user space which can be modified due to access to shared memory. This change scopes the data locally so as to avoid vulnerability of count being modified by external means while executing due to being in shared memory. CRs-Fixed: 3707472 Change-Id: I8a89e23e99b80b089ed4c4cf3098feead752356e Signed-off-by: Shivi Mangal (cherry picked from commit 4e00cc5f9f81bf471d58ee5d6beb210a5326fcff) --- .../cam_sensor_utils/cam_sensor_util.c | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/cam_sensor_module/cam_sensor_utils/cam_sensor_util.c b/drivers/cam_sensor_module/cam_sensor_utils/cam_sensor_util.c index 2f2fe35f81c2..616ba77fcc40 100644 --- a/drivers/cam_sensor_module/cam_sensor_utils/cam_sensor_util.c +++ b/drivers/cam_sensor_module/cam_sensor_utils/cam_sensor_util.c @@ -150,10 +150,11 @@ int32_t cam_sensor_handle_random_write( struct list_head **list) { struct i2c_settings_list *i2c_list; - int32_t rc = 0, cnt; + int32_t rc = 0, cnt, payload_count; + payload_count = cam_cmd_i2c_random_wr->header.count; i2c_list = cam_sensor_get_i2c_ptr(i2c_reg_settings, - cam_cmd_i2c_random_wr->header.count); + payload_count); if (i2c_list == NULL || i2c_list->i2c_settings.reg_setting == NULL) { CAM_ERR(CAM_SENSOR, "Failed in allocating i2c_list"); @@ -162,15 +163,14 @@ int32_t cam_sensor_handle_random_write( *cmd_length_in_bytes = (sizeof(struct i2c_rdwr_header) + sizeof(struct i2c_random_wr_payload) * - (cam_cmd_i2c_random_wr->header.count)); + payload_count); i2c_list->op_code = CAM_SENSOR_I2C_WRITE_RANDOM; i2c_list->i2c_settings.addr_type = cam_cmd_i2c_random_wr->header.addr_type; i2c_list->i2c_settings.data_type = cam_cmd_i2c_random_wr->header.data_type; - for (cnt = 0; cnt < (cam_cmd_i2c_random_wr->header.count); - cnt++) { + for (cnt = 0; cnt < payload_count; cnt++) { i2c_list->i2c_settings.reg_setting[cnt].reg_addr = cam_cmd_i2c_random_wr->random_wr_payload[cnt].reg_addr; i2c_list->i2c_settings.reg_setting[cnt].reg_data = @@ -190,10 +190,11 @@ static int32_t cam_sensor_handle_continuous_write( struct list_head **list) { struct i2c_settings_list *i2c_list; - int32_t rc = 0, cnt; + int32_t rc = 0, cnt, payload_count; + payload_count = cam_cmd_i2c_continuous_wr->header.count; i2c_list = cam_sensor_get_i2c_ptr(i2c_reg_settings, - cam_cmd_i2c_continuous_wr->header.count); + payload_count); if (i2c_list == NULL || i2c_list->i2c_settings.reg_setting == NULL) { CAM_ERR(CAM_SENSOR, "Failed in allocating i2c_list"); @@ -203,7 +204,7 @@ static int32_t cam_sensor_handle_continuous_write( *cmd_length_in_bytes = (sizeof(struct i2c_rdwr_header) + sizeof(cam_cmd_i2c_continuous_wr->reg_addr) + sizeof(struct cam_cmd_read) * - (cam_cmd_i2c_continuous_wr->header.count)); + (payload_count)); if (cam_cmd_i2c_continuous_wr->header.op_code == CAMERA_SENSOR_I2C_OP_CONT_WR_BRST) i2c_list->op_code = CAM_SENSOR_I2C_WRITE_BURST; @@ -220,8 +221,7 @@ static int32_t cam_sensor_handle_continuous_write( i2c_list->i2c_settings.size = cam_cmd_i2c_continuous_wr->header.count; - for (cnt = 0; cnt < (cam_cmd_i2c_continuous_wr->header.count); - cnt++) { + for (cnt = 0; cnt < payload_count; cnt++) { i2c_list->i2c_settings.reg_setting[cnt].reg_addr = cam_cmd_i2c_continuous_wr->reg_addr; i2c_list->i2c_settings.reg_setting[cnt].reg_data = From f49cc667d7160f8c411a7fb489abc788fd843a4a Mon Sep 17 00:00:00 2001 From: zhuo Date: Sat, 6 Apr 2024 17:51:31 +0800 Subject: [PATCH 3/4] msm: camera: memmgr: Add refcount to track umd in use buffers Currently krefcount is using by umd and kmd. Due to sometimes there is issue in umd, such as release twice. That maybe causes buffer release before kmd access the buffer. This commit add a new refcount to track umd in use buffers and use current krefcount to track kmd in use buffers. For the same buffer use in kmd and umd only when all refcount become zero, the buffer start to release. CRs-Fixed: 3692103 Change-Id: I5a58d9bab4c82bdb192d6a6a3d2b3d254dc04c9e Signed-off-by: zhuo --- drivers/cam_req_mgr/cam_mem_mgr.c | 114 +++++++++++++++++++++++++----- drivers/cam_req_mgr/cam_mem_mgr.h | 9 ++- 2 files changed, 105 insertions(+), 18 deletions(-) diff --git a/drivers/cam_req_mgr/cam_mem_mgr.c b/drivers/cam_req_mgr/cam_mem_mgr.c index 9d7b68921e8f..229a5740d46f 100644 --- a/drivers/cam_req_mgr/cam_mem_mgr.c +++ b/drivers/cam_req_mgr/cam_mem_mgr.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2016-2020 The Linux Foundation. All rights reserved. - * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -187,6 +187,7 @@ static int32_t cam_mem_get_slot(void) set_bit(idx, tbl.bitmap); tbl.bufq[idx].active = true; mutex_init(&tbl.bufq[idx].q_lock); + mutex_init(&tbl.bufq[idx].ref_lock); mutex_unlock(&tbl.m_lock); return idx; @@ -198,7 +199,12 @@ static void cam_mem_put_slot(int32_t idx) mutex_lock(&tbl.bufq[idx].q_lock); tbl.bufq[idx].active = false; mutex_unlock(&tbl.bufq[idx].q_lock); + mutex_lock(&tbl.bufq[idx].ref_lock); + memset(&tbl.bufq[idx].krefcount, 0, sizeof(struct kref)); + memset(&tbl.bufq[idx].urefcount, 0, sizeof(struct kref)); + mutex_unlock(&tbl.bufq[idx].ref_lock); mutex_destroy(&tbl.bufq[idx].q_lock); + mutex_destroy(&tbl.bufq[idx].ref_lock); clear_bit(idx, tbl.bitmap); mutex_unlock(&tbl.m_lock); } @@ -291,16 +297,19 @@ int cam_mem_get_cpu_buf(int32_t buf_handle, uintptr_t *vaddr_ptr, size_t *len) return -EINVAL; } + mutex_lock(&tbl.bufq[idx].ref_lock); if (tbl.bufq[idx].kmdvaddr && kref_get_unless_zero(&tbl.bufq[idx].krefcount)) { *vaddr_ptr = tbl.bufq[idx].kmdvaddr; *len = tbl.bufq[idx].len; } else { + mutex_unlock(&tbl.bufq[idx].ref_lock); CAM_ERR(CAM_MEM, "No KMD access request, vddr= %p, idx= %d, handle= %d", tbl.bufq[idx].kmdvaddr, idx, buf_handle); return -EINVAL; } + mutex_unlock(&tbl.bufq[idx].ref_lock); return 0; } @@ -718,7 +727,12 @@ int cam_mem_mgr_alloc_and_map(struct cam_mem_mgr_alloc_cmd *cmd) memcpy(tbl.bufq[idx].hdls, cmd->mmu_hdls, sizeof(int32_t) * cmd->num_hdl); tbl.bufq[idx].is_imported = false; - kref_init(&tbl.bufq[idx].krefcount); + + if (cmd->flags & CAM_MEM_FLAG_KMD_ACCESS) + kref_init(&tbl.bufq[idx].krefcount); + + kref_init(&tbl.bufq[idx].urefcount); + tbl.bufq[idx].smmu_mapping_client = CAM_SMMU_MAPPING_USER; mutex_unlock(&tbl.bufq[idx].q_lock); @@ -822,7 +836,9 @@ int cam_mem_mgr_map(struct cam_mem_mgr_map_cmd *cmd) memcpy(tbl.bufq[idx].hdls, cmd->mmu_hdls, sizeof(int32_t) * cmd->num_hdl); tbl.bufq[idx].is_imported = true; - kref_init(&tbl.bufq[idx].krefcount); + if (cmd->flags & CAM_MEM_FLAG_KMD_ACCESS) + kref_init(&tbl.bufq[idx].krefcount); + kref_init(&tbl.bufq[idx].urefcount); tbl.bufq[idx].smmu_mapping_client = CAM_SMMU_MAPPING_USER; mutex_unlock(&tbl.bufq[idx].q_lock); @@ -951,7 +967,12 @@ static int cam_mem_mgr_cleanup_table(void) tbl.bufq[i].dma_buf = NULL; tbl.bufq[i].active = false; mutex_unlock(&tbl.bufq[i].q_lock); + mutex_lock(&tbl.bufq[i].ref_lock); + memset(&tbl.bufq[i].krefcount, 0, sizeof(struct kref)); + memset(&tbl.bufq[i].urefcount, 0, sizeof(struct kref)); + mutex_unlock(&tbl.bufq[i].ref_lock); mutex_destroy(&tbl.bufq[i].q_lock); + mutex_destroy(&tbl.bufq[i].ref_lock); } bitmap_zero(tbl.bitmap, tbl.bits); @@ -975,16 +996,17 @@ void cam_mem_mgr_deinit(void) mutex_destroy(&tbl.m_lock); } -static void cam_mem_util_unmap(struct kref *kref) +static void cam_mem_util_unmap_dummy(struct kref *kref) +{ + CAM_DBG(CAM_MEM, "Cam mem util unmap dummy"); +} + +static void cam_mem_util_unmap(int32_t idx) { int rc = 0; - int32_t idx; enum cam_smmu_region_id region = CAM_SMMU_REGION_SHARED; enum cam_smmu_mapping_client client; - struct cam_mem_buf_queue *bufq = - container_of(kref, typeof(*bufq), krefcount); - idx = CAM_MEM_MGR_GET_HDL_IDX(bufq->buf_handle); if (idx >= CAM_MEM_BUFQ_MAX || idx <= 0) { CAM_ERR(CAM_MEM, "Incorrect index"); return; @@ -1055,6 +1077,8 @@ static void cam_mem_util_unmap(struct kref *kref) tbl.bufq[idx].len = 0; tbl.bufq[idx].num_hdl = 0; tbl.bufq[idx].active = false; + memset(&tbl.bufq[idx].krefcount, 0, sizeof(struct kref)); + memset(&tbl.bufq[idx].urefcount, 0, sizeof(struct kref)); mutex_unlock(&tbl.bufq[idx].q_lock); mutex_destroy(&tbl.bufq[idx].q_lock); clear_bit(idx, tbl.bitmap); @@ -1062,10 +1086,28 @@ static void cam_mem_util_unmap(struct kref *kref) } +static void cam_mem_util_unmap_wrapper(struct kref *kref) +{ + int32_t idx; + struct cam_mem_buf_queue *bufq = container_of(kref, typeof(*bufq), krefcount); + + idx = CAM_MEM_MGR_GET_HDL_IDX(bufq->buf_handle); + if (idx >= CAM_MEM_BUFQ_MAX || idx <= 0) { + CAM_ERR(CAM_MEM, "idx: %d not valid", idx); + return; + } + + cam_mem_util_unmap(idx); + + mutex_destroy(&tbl.bufq[idx].ref_lock); +} + void cam_mem_put_cpu_buf(int32_t buf_handle) { int rc = 0; int idx; + uint32_t krefcount = 0, urefcount = 0; + bool unmap = false; if (!buf_handle) { CAM_ERR(CAM_MEM, "Invalid buf_handle"); @@ -1091,10 +1133,28 @@ void cam_mem_put_cpu_buf(int32_t buf_handle) return; } - if (kref_put(&tbl.bufq[idx].krefcount, cam_mem_util_unmap)) + mutex_lock(&tbl.bufq[idx].ref_lock); + kref_put(&tbl.bufq[idx].krefcount, cam_mem_util_unmap_dummy); + + krefcount = kref_read(&tbl.bufq[idx].krefcount); + urefcount = kref_read(&tbl.bufq[idx].urefcount); + + if ((krefcount == 1) && (urefcount == 0)) + unmap = true; + + if (unmap) { + cam_mem_util_unmap(idx); CAM_DBG(CAM_MEM, - "Called unmap from here, buf_handle: %u, idx: %d", - buf_handle, idx); + "Called unmap from here, buf_handle: %u, idx: %d", buf_handle, idx); + } else if (krefcount == 0) { + CAM_ERR(CAM_MEM, + "Unbalanced release Called buf_handle: %u, idx: %d", + tbl.bufq[idx].buf_handle, idx); + } + mutex_unlock(&tbl.bufq[idx].ref_lock); + + if (unmap) + mutex_destroy(&tbl.bufq[idx].ref_lock); } EXPORT_SYMBOL(cam_mem_put_cpu_buf); @@ -1104,6 +1164,8 @@ int cam_mem_mgr_release(struct cam_mem_mgr_release_cmd *cmd) { int idx; int rc = 0; + uint32_t krefcount = 0, urefcount = 0; + bool unmap = false; if (!atomic_read(&cam_mem_mgr_state)) { CAM_ERR(CAM_MEM, "failed. mem_mgr not initialized"); @@ -1136,10 +1198,30 @@ int cam_mem_mgr_release(struct cam_mem_mgr_release_cmd *cmd) CAM_DBG(CAM_MEM, "Releasing hdl = %x, idx = %d", cmd->buf_handle, idx); - if (kref_put(&tbl.bufq[idx].krefcount, cam_mem_util_unmap)) + mutex_lock(&tbl.bufq[idx].ref_lock); + kref_put(&tbl.bufq[idx].urefcount, cam_mem_util_unmap_dummy); + + urefcount = kref_read(&tbl.bufq[idx].urefcount); + + if (tbl.bufq[idx].flags & CAM_MEM_FLAG_KMD_ACCESS) { + krefcount = kref_read(&tbl.bufq[idx].krefcount); + if ((krefcount == 1) && (urefcount == 0)) + unmap = true; + } else { + if (urefcount == 0) + unmap = true; + } + + if (unmap) { + cam_mem_util_unmap(idx); CAM_DBG(CAM_MEM, - "Called unmap from here, buf_handle: %u, idx: %d", - cmd->buf_handle, idx); + "Called unmap from here, buf_handle: %u, idx: %d", cmd->buf_handle, idx); + } + + mutex_unlock(&tbl.bufq[idx].ref_lock); + + if (unmap) + mutex_destroy(&tbl.bufq[idx].ref_lock); return rc; } @@ -1321,7 +1403,7 @@ int cam_mem_mgr_release_mem(struct cam_mem_mgr_memory_desc *inp) } CAM_DBG(CAM_MEM, "Releasing hdl = %X", inp->mem_handle); - if (kref_put(&tbl.bufq[idx].krefcount, cam_mem_util_unmap)) + if (kref_put(&tbl.bufq[idx].krefcount, cam_mem_util_unmap_wrapper)) CAM_DBG(CAM_MEM, "Called unmap from here, buf_handle: %u, idx: %d", tbl.bufq[idx].buf_handle, idx); @@ -1501,7 +1583,7 @@ int cam_mem_mgr_free_memory_region(struct cam_mem_mgr_memory_desc *inp) } CAM_DBG(CAM_MEM, "Releasing hdl = %X", inp->mem_handle); - if (kref_put(&tbl.bufq[idx].krefcount, cam_mem_util_unmap)) + if (kref_put(&tbl.bufq[idx].krefcount, cam_mem_util_unmap_wrapper)) CAM_DBG(CAM_MEM, "Called unmap from here, buf_handle: %u, idx: %d", inp->mem_handle, idx); diff --git a/drivers/cam_req_mgr/cam_mem_mgr.h b/drivers/cam_req_mgr/cam_mem_mgr.h index 84b3ce43eb81..856de789b180 100644 --- a/drivers/cam_req_mgr/cam_mem_mgr.h +++ b/drivers/cam_req_mgr/cam_mem_mgr.h @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved. - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved. */ #ifndef _CAM_MEM_MGR_H_ @@ -44,8 +44,11 @@ enum cam_smmu_mapping_client { * @is_imported: Flag indicating if buffer is imported from an FD * in user space * @krefcount: Reference counter to track whether the buffer is - * mapped and in use + * mapped and in use by kmd * @smmu_mapping_client: Client buffer (User or kernel) + * @urefcount: Reference counter to track whether the buffer is + * mapped and in use by umd + * @ref_lock: Mutex lock for refcount */ struct cam_mem_buf_queue { struct dma_buf *dma_buf; @@ -63,6 +66,8 @@ struct cam_mem_buf_queue { bool is_imported; struct kref krefcount; enum cam_smmu_mapping_client smmu_mapping_client; + struct kref urefcount; + struct mutex ref_lock; }; /** From 58305ce1ff466f5dd2108b3a2ccef76b389107cd Mon Sep 17 00:00:00 2001 From: Depeng Shao Date: Mon, 15 Apr 2024 12:43:55 +0530 Subject: [PATCH 4/4] msm: camera: memmgr: Remove the mutex lock for kref variable kref operation is atmoic operation, so no need to use mutex to protect it, and the cam_mem_put_cpu_buf is also called in spinlock context, so we can't use mutex lock in this function. This change removes the mutex lock for kref variable protextion. CRs-Fixed: 3786887 Change-Id: Ic05bdafacf06cde6a8d8dbae7512e5d22eb7f514 Signed-off-by: Depeng Shao --- drivers/cam_req_mgr/cam_mem_mgr.c | 32 ++++--------------------------- drivers/cam_req_mgr/cam_mem_mgr.h | 2 -- 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/drivers/cam_req_mgr/cam_mem_mgr.c b/drivers/cam_req_mgr/cam_mem_mgr.c index 229a5740d46f..dcadab67be7f 100644 --- a/drivers/cam_req_mgr/cam_mem_mgr.c +++ b/drivers/cam_req_mgr/cam_mem_mgr.c @@ -187,7 +187,6 @@ static int32_t cam_mem_get_slot(void) set_bit(idx, tbl.bitmap); tbl.bufq[idx].active = true; mutex_init(&tbl.bufq[idx].q_lock); - mutex_init(&tbl.bufq[idx].ref_lock); mutex_unlock(&tbl.m_lock); return idx; @@ -198,13 +197,10 @@ static void cam_mem_put_slot(int32_t idx) mutex_lock(&tbl.m_lock); mutex_lock(&tbl.bufq[idx].q_lock); tbl.bufq[idx].active = false; + kref_init(&tbl.bufq[idx].krefcount); + kref_init(&tbl.bufq[idx].urefcount); mutex_unlock(&tbl.bufq[idx].q_lock); - mutex_lock(&tbl.bufq[idx].ref_lock); - memset(&tbl.bufq[idx].krefcount, 0, sizeof(struct kref)); - memset(&tbl.bufq[idx].urefcount, 0, sizeof(struct kref)); - mutex_unlock(&tbl.bufq[idx].ref_lock); mutex_destroy(&tbl.bufq[idx].q_lock); - mutex_destroy(&tbl.bufq[idx].ref_lock); clear_bit(idx, tbl.bitmap); mutex_unlock(&tbl.m_lock); } @@ -297,19 +293,16 @@ int cam_mem_get_cpu_buf(int32_t buf_handle, uintptr_t *vaddr_ptr, size_t *len) return -EINVAL; } - mutex_lock(&tbl.bufq[idx].ref_lock); if (tbl.bufq[idx].kmdvaddr && kref_get_unless_zero(&tbl.bufq[idx].krefcount)) { *vaddr_ptr = tbl.bufq[idx].kmdvaddr; *len = tbl.bufq[idx].len; } else { - mutex_unlock(&tbl.bufq[idx].ref_lock); CAM_ERR(CAM_MEM, "No KMD access request, vddr= %p, idx= %d, handle= %d", tbl.bufq[idx].kmdvaddr, idx, buf_handle); return -EINVAL; } - mutex_unlock(&tbl.bufq[idx].ref_lock); return 0; } @@ -966,13 +959,10 @@ static int cam_mem_mgr_cleanup_table(void) tbl.bufq[i].num_hdl = 0; tbl.bufq[i].dma_buf = NULL; tbl.bufq[i].active = false; + kref_init(&tbl.bufq[i].krefcount); + kref_init(&tbl.bufq[i].urefcount); mutex_unlock(&tbl.bufq[i].q_lock); - mutex_lock(&tbl.bufq[i].ref_lock); - memset(&tbl.bufq[i].krefcount, 0, sizeof(struct kref)); - memset(&tbl.bufq[i].urefcount, 0, sizeof(struct kref)); - mutex_unlock(&tbl.bufq[i].ref_lock); mutex_destroy(&tbl.bufq[i].q_lock); - mutex_destroy(&tbl.bufq[i].ref_lock); } bitmap_zero(tbl.bitmap, tbl.bits); @@ -1098,8 +1088,6 @@ static void cam_mem_util_unmap_wrapper(struct kref *kref) } cam_mem_util_unmap(idx); - - mutex_destroy(&tbl.bufq[idx].ref_lock); } void cam_mem_put_cpu_buf(int32_t buf_handle) @@ -1133,7 +1121,6 @@ void cam_mem_put_cpu_buf(int32_t buf_handle) return; } - mutex_lock(&tbl.bufq[idx].ref_lock); kref_put(&tbl.bufq[idx].krefcount, cam_mem_util_unmap_dummy); krefcount = kref_read(&tbl.bufq[idx].krefcount); @@ -1151,11 +1138,6 @@ void cam_mem_put_cpu_buf(int32_t buf_handle) "Unbalanced release Called buf_handle: %u, idx: %d", tbl.bufq[idx].buf_handle, idx); } - mutex_unlock(&tbl.bufq[idx].ref_lock); - - if (unmap) - mutex_destroy(&tbl.bufq[idx].ref_lock); - } EXPORT_SYMBOL(cam_mem_put_cpu_buf); @@ -1198,7 +1180,6 @@ int cam_mem_mgr_release(struct cam_mem_mgr_release_cmd *cmd) CAM_DBG(CAM_MEM, "Releasing hdl = %x, idx = %d", cmd->buf_handle, idx); - mutex_lock(&tbl.bufq[idx].ref_lock); kref_put(&tbl.bufq[idx].urefcount, cam_mem_util_unmap_dummy); urefcount = kref_read(&tbl.bufq[idx].urefcount); @@ -1218,11 +1199,6 @@ int cam_mem_mgr_release(struct cam_mem_mgr_release_cmd *cmd) "Called unmap from here, buf_handle: %u, idx: %d", cmd->buf_handle, idx); } - mutex_unlock(&tbl.bufq[idx].ref_lock); - - if (unmap) - mutex_destroy(&tbl.bufq[idx].ref_lock); - return rc; } diff --git a/drivers/cam_req_mgr/cam_mem_mgr.h b/drivers/cam_req_mgr/cam_mem_mgr.h index 856de789b180..cdaad9a1e22a 100644 --- a/drivers/cam_req_mgr/cam_mem_mgr.h +++ b/drivers/cam_req_mgr/cam_mem_mgr.h @@ -48,7 +48,6 @@ enum cam_smmu_mapping_client { * @smmu_mapping_client: Client buffer (User or kernel) * @urefcount: Reference counter to track whether the buffer is * mapped and in use by umd - * @ref_lock: Mutex lock for refcount */ struct cam_mem_buf_queue { struct dma_buf *dma_buf; @@ -67,7 +66,6 @@ struct cam_mem_buf_queue { struct kref krefcount; enum cam_smmu_mapping_client smmu_mapping_client; struct kref urefcount; - struct mutex ref_lock; }; /**