From 27597eb95bf4c5f089d24afb990f007f666dba0b Mon Sep 17 00:00:00 2001 From: Shivi Mangal Date: Thu, 21 Mar 2024 17:36:40 -0700 Subject: [PATCH 1/3] 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) (cherry picked from commit 6245be68a914dd9984c259ab3c6bc1647c1593be) --- .../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 b4da51bfee99..fd423ef3da0a 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 @@ -195,10 +195,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"); @@ -207,15 +208,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 = @@ -235,10 +235,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"); @@ -248,7 +249,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; @@ -265,8 +266,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 010154959c107093342506ec7b40fb399629bac7 Mon Sep 17 00:00:00 2001 From: zhuo Date: Sun, 7 Apr 2024 17:12:15 +0800 Subject: [PATCH 2/3] 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 | 130 +++++++++++++++++++++++++----- drivers/cam_req_mgr/cam_mem_mgr.h | 7 +- 2 files changed, 114 insertions(+), 23 deletions(-) diff --git a/drivers/cam_req_mgr/cam_mem_mgr.c b/drivers/cam_req_mgr/cam_mem_mgr.c index 9e2efb911cbd..3273b4d3c350 100644 --- a/drivers/cam_req_mgr/cam_mem_mgr.c +++ b/drivers/cam_req_mgr/cam_mem_mgr.c @@ -36,9 +36,10 @@ static void cam_mem_mgr_print_tbl(void) for (i = 1; i < CAM_MEM_BUFQ_MAX; i++) { CAM_CONVERT_TIMESTAMP_FORMAT((tbl.bufq[i].timestamp), hrs, min, sec, ms); CAM_INFO(CAM_MEM, - "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d refCount %d", + "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d krefCount %d urefCount %d", hrs, min, sec, ms, i, tbl.bufq[i].fd, tbl.bufq[i].len, tbl.bufq[i].active, - tbl.bufq[i].buf_handle, kref_read(&tbl.bufq[i].krefcount)); + tbl.bufq[i].buf_handle, kref_read(&tbl.bufq[i].krefcount), + kref_read(&tbl.bufq[i].urefcount)); } } @@ -198,6 +199,7 @@ static int32_t cam_mem_get_slot(void) tbl.bufq[idx].release_deferred = false; CAM_GET_TIMESTAMP((tbl.bufq[idx].timestamp)); mutex_init(&tbl.bufq[idx].q_lock); + mutex_init(&tbl.bufq[idx].ref_lock); mutex_unlock(&tbl.m_lock); return idx; @@ -212,7 +214,12 @@ static void cam_mem_put_slot(int32_t idx) tbl.bufq[idx].is_internal = false; memset(&tbl.bufq[idx].timestamp, 0, sizeof(struct timespec64)); 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); } @@ -311,14 +318,17 @@ 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 requested, kmdvddr= %p, idx= %d, buf_handle= %d", tbl.bufq[idx].kmdvaddr, idx, buf_handle); return -EINVAL; } + mutex_unlock(&tbl.bufq[idx].ref_lock); return 0; } @@ -758,7 +768,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); @@ -894,7 +909,9 @@ int cam_mem_mgr_map(struct cam_mem_mgr_map_cmd *cmd) sizeof(int32_t) * cmd->num_hdl); tbl.bufq[idx].is_imported = true; tbl.bufq[idx].is_internal = is_internal; - 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); @@ -1030,7 +1047,12 @@ static int cam_mem_mgr_cleanup_table(void) tbl.bufq[i].release_deferred = false; tbl.bufq[i].is_internal = 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); @@ -1055,16 +1077,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; @@ -1141,6 +1164,8 @@ static void cam_mem_util_unmap(struct kref *kref) tbl.bufq[idx].len = 0; tbl.bufq[idx].num_hdl = 0; memset(&tbl.bufq[idx].timestamp, 0, sizeof(struct timespec64)); + 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); @@ -1148,12 +1173,30 @@ 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; uint64_t ms, hrs, min, sec; struct timespec64 current_ts; + uint32_t krefcount = 0, urefcount = 0; + bool unmap = false; if (!buf_handle) { CAM_ERR(CAM_MEM, "Invalid buf_handle"); @@ -1179,7 +1222,17 @@ 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_GET_TIMESTAMP(current_ts); CAM_CONVERT_TIMESTAMP_FORMAT(current_ts, hrs, min, sec, ms); CAM_DBG(CAM_MEM, @@ -1188,16 +1241,24 @@ void cam_mem_put_cpu_buf(int32_t buf_handle) } else if (tbl.bufq[idx].release_deferred) { CAM_CONVERT_TIMESTAMP_FORMAT((tbl.bufq[idx].timestamp), hrs, min, sec, ms); CAM_ERR(CAM_MEM, - "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d refCount %d", + "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d krefCount %d urefCount %d", hrs, min, sec, ms, idx, tbl.bufq[idx].fd, tbl.bufq[idx].len, - tbl.bufq[idx].active, tbl.bufq[idx].buf_handle, - kref_read(&tbl.bufq[idx].krefcount)); + tbl.bufq[idx].active, tbl.bufq[idx].buf_handle, krefcount, urefcount); CAM_GET_TIMESTAMP(current_ts); CAM_CONVERT_TIMESTAMP_FORMAT(current_ts, hrs, min, sec, ms); CAM_ERR(CAM_MEM, "%llu:%llu:%llu:%llu Not unmapping even after defer, buf_handle: %u, idx: %d", hrs, min, sec, ms, 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); @@ -1208,6 +1269,8 @@ int cam_mem_mgr_release(struct cam_mem_mgr_release_cmd *cmd) int rc = 0; uint64_t ms, hrs, min, sec; struct timespec64 current_ts; + 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"); @@ -1239,22 +1302,45 @@ 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)) { - CAM_DBG(CAM_MEM, - "Called unmap from here, buf_handle: %u, 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); + + 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); + } else if (tbl.bufq[idx].flags & CAM_MEM_FLAG_KMD_ACCESS) { rc = -EINVAL; CAM_GET_TIMESTAMP(current_ts); CAM_CONVERT_TIMESTAMP_FORMAT(current_ts, hrs, min, sec, ms); CAM_CONVERT_TIMESTAMP_FORMAT((tbl.bufq[idx].timestamp), hrs, min, sec, ms); CAM_ERR(CAM_MEM, - "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d refCount %d", + "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d krefCount %d urefCount %d", hrs, min, sec, ms, idx, tbl.bufq[idx].fd, tbl.bufq[idx].len, - tbl.bufq[idx].active, tbl.bufq[idx].buf_handle, - kref_read(&tbl.bufq[idx].krefcount)); + tbl.bufq[idx].active, tbl.bufq[idx].buf_handle, krefcount, urefcount); + if (tbl.bufq[idx].release_deferred) + CAM_ERR(CAM_MEM, "Unbalanced release Called buf_handle: %u, idx: %d", + tbl.bufq[idx].buf_handle, idx); tbl.bufq[idx].release_deferred = true; } + + mutex_unlock(&tbl.bufq[idx].ref_lock); + + if (unmap) + mutex_destroy(&tbl.bufq[idx].ref_lock); + return rc; } @@ -1437,7 +1523,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); @@ -1619,7 +1705,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 2a78827271ad..0e31a4269359 100644 --- a/drivers/cam_req_mgr/cam_mem_mgr.h +++ b/drivers/cam_req_mgr/cam_mem_mgr.h @@ -44,8 +44,11 @@ enum cam_smmu_mapping_client { * @is_internal: Flag indicating kernel allocated buffer * @timestamp: Timestamp at which this entry in tbl was made * @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; @@ -66,6 +69,8 @@ struct cam_mem_buf_queue { struct timespec64 timestamp; struct kref krefcount; enum cam_smmu_mapping_client smmu_mapping_client; + struct kref urefcount; + struct mutex ref_lock; }; /** From 1d079b356c9f7f782223664ec3a977c330506c27 Mon Sep 17 00:00:00 2001 From: zhuo Date: Sun, 7 Apr 2024 17:12:15 +0800 Subject: [PATCH 3/3] 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 (cherry picked from commit 010154959c107093342506ec7b40fb399629bac7) --- drivers/cam_req_mgr/cam_mem_mgr.c | 130 +++++++++++++++++++++++++----- drivers/cam_req_mgr/cam_mem_mgr.h | 7 +- 2 files changed, 114 insertions(+), 23 deletions(-) diff --git a/drivers/cam_req_mgr/cam_mem_mgr.c b/drivers/cam_req_mgr/cam_mem_mgr.c index 9e2efb911cbd..3273b4d3c350 100644 --- a/drivers/cam_req_mgr/cam_mem_mgr.c +++ b/drivers/cam_req_mgr/cam_mem_mgr.c @@ -36,9 +36,10 @@ static void cam_mem_mgr_print_tbl(void) for (i = 1; i < CAM_MEM_BUFQ_MAX; i++) { CAM_CONVERT_TIMESTAMP_FORMAT((tbl.bufq[i].timestamp), hrs, min, sec, ms); CAM_INFO(CAM_MEM, - "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d refCount %d", + "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d krefCount %d urefCount %d", hrs, min, sec, ms, i, tbl.bufq[i].fd, tbl.bufq[i].len, tbl.bufq[i].active, - tbl.bufq[i].buf_handle, kref_read(&tbl.bufq[i].krefcount)); + tbl.bufq[i].buf_handle, kref_read(&tbl.bufq[i].krefcount), + kref_read(&tbl.bufq[i].urefcount)); } } @@ -198,6 +199,7 @@ static int32_t cam_mem_get_slot(void) tbl.bufq[idx].release_deferred = false; CAM_GET_TIMESTAMP((tbl.bufq[idx].timestamp)); mutex_init(&tbl.bufq[idx].q_lock); + mutex_init(&tbl.bufq[idx].ref_lock); mutex_unlock(&tbl.m_lock); return idx; @@ -212,7 +214,12 @@ static void cam_mem_put_slot(int32_t idx) tbl.bufq[idx].is_internal = false; memset(&tbl.bufq[idx].timestamp, 0, sizeof(struct timespec64)); 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); } @@ -311,14 +318,17 @@ 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 requested, kmdvddr= %p, idx= %d, buf_handle= %d", tbl.bufq[idx].kmdvaddr, idx, buf_handle); return -EINVAL; } + mutex_unlock(&tbl.bufq[idx].ref_lock); return 0; } @@ -758,7 +768,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); @@ -894,7 +909,9 @@ int cam_mem_mgr_map(struct cam_mem_mgr_map_cmd *cmd) sizeof(int32_t) * cmd->num_hdl); tbl.bufq[idx].is_imported = true; tbl.bufq[idx].is_internal = is_internal; - 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); @@ -1030,7 +1047,12 @@ static int cam_mem_mgr_cleanup_table(void) tbl.bufq[i].release_deferred = false; tbl.bufq[i].is_internal = 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); @@ -1055,16 +1077,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; @@ -1141,6 +1164,8 @@ static void cam_mem_util_unmap(struct kref *kref) tbl.bufq[idx].len = 0; tbl.bufq[idx].num_hdl = 0; memset(&tbl.bufq[idx].timestamp, 0, sizeof(struct timespec64)); + 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); @@ -1148,12 +1173,30 @@ 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; uint64_t ms, hrs, min, sec; struct timespec64 current_ts; + uint32_t krefcount = 0, urefcount = 0; + bool unmap = false; if (!buf_handle) { CAM_ERR(CAM_MEM, "Invalid buf_handle"); @@ -1179,7 +1222,17 @@ 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_GET_TIMESTAMP(current_ts); CAM_CONVERT_TIMESTAMP_FORMAT(current_ts, hrs, min, sec, ms); CAM_DBG(CAM_MEM, @@ -1188,16 +1241,24 @@ void cam_mem_put_cpu_buf(int32_t buf_handle) } else if (tbl.bufq[idx].release_deferred) { CAM_CONVERT_TIMESTAMP_FORMAT((tbl.bufq[idx].timestamp), hrs, min, sec, ms); CAM_ERR(CAM_MEM, - "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d refCount %d", + "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d krefCount %d urefCount %d", hrs, min, sec, ms, idx, tbl.bufq[idx].fd, tbl.bufq[idx].len, - tbl.bufq[idx].active, tbl.bufq[idx].buf_handle, - kref_read(&tbl.bufq[idx].krefcount)); + tbl.bufq[idx].active, tbl.bufq[idx].buf_handle, krefcount, urefcount); CAM_GET_TIMESTAMP(current_ts); CAM_CONVERT_TIMESTAMP_FORMAT(current_ts, hrs, min, sec, ms); CAM_ERR(CAM_MEM, "%llu:%llu:%llu:%llu Not unmapping even after defer, buf_handle: %u, idx: %d", hrs, min, sec, ms, 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); @@ -1208,6 +1269,8 @@ int cam_mem_mgr_release(struct cam_mem_mgr_release_cmd *cmd) int rc = 0; uint64_t ms, hrs, min, sec; struct timespec64 current_ts; + 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"); @@ -1239,22 +1302,45 @@ 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)) { - CAM_DBG(CAM_MEM, - "Called unmap from here, buf_handle: %u, 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); + + 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); + } else if (tbl.bufq[idx].flags & CAM_MEM_FLAG_KMD_ACCESS) { rc = -EINVAL; CAM_GET_TIMESTAMP(current_ts); CAM_CONVERT_TIMESTAMP_FORMAT(current_ts, hrs, min, sec, ms); CAM_CONVERT_TIMESTAMP_FORMAT((tbl.bufq[idx].timestamp), hrs, min, sec, ms); CAM_ERR(CAM_MEM, - "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d refCount %d", + "%llu:%llu:%llu:%llu idx %d fd %d size %llu active %d buf_handle %d krefCount %d urefCount %d", hrs, min, sec, ms, idx, tbl.bufq[idx].fd, tbl.bufq[idx].len, - tbl.bufq[idx].active, tbl.bufq[idx].buf_handle, - kref_read(&tbl.bufq[idx].krefcount)); + tbl.bufq[idx].active, tbl.bufq[idx].buf_handle, krefcount, urefcount); + if (tbl.bufq[idx].release_deferred) + CAM_ERR(CAM_MEM, "Unbalanced release Called buf_handle: %u, idx: %d", + tbl.bufq[idx].buf_handle, idx); tbl.bufq[idx].release_deferred = true; } + + mutex_unlock(&tbl.bufq[idx].ref_lock); + + if (unmap) + mutex_destroy(&tbl.bufq[idx].ref_lock); + return rc; } @@ -1437,7 +1523,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); @@ -1619,7 +1705,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 2a78827271ad..0e31a4269359 100644 --- a/drivers/cam_req_mgr/cam_mem_mgr.h +++ b/drivers/cam_req_mgr/cam_mem_mgr.h @@ -44,8 +44,11 @@ enum cam_smmu_mapping_client { * @is_internal: Flag indicating kernel allocated buffer * @timestamp: Timestamp at which this entry in tbl was made * @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; @@ -66,6 +69,8 @@ struct cam_mem_buf_queue { struct timespec64 timestamp; struct kref krefcount; enum cam_smmu_mapping_client smmu_mapping_client; + struct kref urefcount; + struct mutex ref_lock; }; /**