From 320ae57bb915e0e91be7546bb13fd0458a48f210 Mon Sep 17 00:00:00 2001 From: Ayush Kumar Date: Tue, 4 Feb 2020 11:33:11 +0530 Subject: [PATCH] msm: camera: core: Fix context release timing issue When sync object is invalid and num_in_map > 1, it will lead to context ref leak. Check sync object first, then register sync call back. CRs-Fixed: 2594185 Change-Id: I2d39ce3ea43bbe7bc05420b86b37fdfba4aa795a Signed-off-by: Ayush Kumar --- drivers/cam_core/cam_context_utils.c | 19 +++++++++++----- drivers/cam_sync/cam_sync.c | 33 ++++++++++++++++++++++++++++ drivers/cam_sync/cam_sync_api.h | 12 +++++++++- 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/drivers/cam_core/cam_context_utils.c b/drivers/cam_core/cam_context_utils.c index 87abebabaf..09f164561d 100644 --- a/drivers/cam_core/cam_context_utils.c +++ b/drivers/cam_core/cam_context_utils.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. */ #include @@ -451,6 +451,17 @@ int32_t cam_context_prepare_dev_to_hw(struct cam_context *ctx, "[%s][%d] : Moving req[%llu] from free_list to pending_list", ctx->dev_name, ctx->ctx_id, req->request_id); + for (j = 0; j < req->num_in_map_entries; j++) { + rc = cam_sync_check_valid( + req->in_map_entries[j].sync_id); + if (rc) { + CAM_ERR(CAM_CTXT, + "invalid in map sync object %d", + req->in_map_entries[j].sync_id); + goto put_ref; + } + } + for (j = 0; j < req->num_in_map_entries; j++) { cam_context_getref(ctx); rc = cam_sync_register_callback( @@ -472,7 +483,8 @@ int32_t cam_context_prepare_dev_to_hw(struct cam_context *ctx, ctx->dev_name, ctx->ctx_id, req->request_id); - goto put_ctx_ref; + cam_context_putref(ctx); + goto put_ref; } CAM_DBG(CAM_CTXT, "register in fence cb: %d ret = %d", req->in_map_entries[j].sync_id, rc); @@ -480,9 +492,6 @@ int32_t cam_context_prepare_dev_to_hw(struct cam_context *ctx, } return rc; -put_ctx_ref: - for (; j >= 0; j--) - cam_context_putref(ctx); put_ref: for (--i; i >= 0; i--) { if (cam_sync_put_obj_ref(req->out_map_entries[i].sync_id)) diff --git a/drivers/cam_sync/cam_sync.c b/drivers/cam_sync/cam_sync.c index 8f7d9d8389..7f6a38a283 100644 --- a/drivers/cam_sync/cam_sync.c +++ b/drivers/cam_sync/cam_sync.c @@ -287,6 +287,7 @@ int cam_sync_merge(int32_t *sync_obj, uint32_t num_objs, int32_t *merged_obj) int rc; long idx = 0; bool bit; + int i = 0; if (!sync_obj || !merged_obj) { CAM_ERR(CAM_SYNC, "Invalid pointer(s)"); @@ -304,6 +305,14 @@ int cam_sync_merge(int32_t *sync_obj, uint32_t num_objs, int32_t *merged_obj) return -EINVAL; } + for (i = 0; i < num_objs; i++) { + rc = cam_sync_check_valid(sync_obj[i]); + if (rc) { + CAM_ERR(CAM_SYNC, "Sync_obj[%d] %d valid check fail", + i, sync_obj[i]); + return rc; + } + } do { idx = find_first_zero_bit(sync_dev->bitmap, CAM_SYNC_MAX_OBJS); if (idx >= CAM_SYNC_MAX_OBJS) @@ -375,6 +384,30 @@ int cam_sync_destroy(int32_t sync_obj) return cam_sync_deinit_object(sync_dev->sync_table, sync_obj); } +int cam_sync_check_valid(int32_t sync_obj) +{ + struct sync_table_row *row = NULL; + + if (sync_obj >= CAM_SYNC_MAX_OBJS || sync_obj <= 0) + return -EINVAL; + + row = sync_dev->sync_table + sync_obj; + + if (!test_bit(sync_obj, sync_dev->bitmap)) { + CAM_ERR(CAM_SYNC, "Error: Released sync obj received %d", + sync_obj); + return -EINVAL; + } + + if (row->state == CAM_SYNC_STATE_INVALID) { + CAM_ERR(CAM_SYNC, + "Error: accessing an uninitialized sync obj = %d", + sync_obj); + return -EINVAL; + } + return 0; +} + int cam_sync_wait(int32_t sync_obj, uint64_t timeout_ms) { unsigned long timeleft; diff --git a/drivers/cam_sync/cam_sync_api.h b/drivers/cam_sync/cam_sync_api.h index bce22226f7..a46e37b31b 100644 --- a/drivers/cam_sync/cam_sync_api.h +++ b/drivers/cam_sync/cam_sync_api.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. */ #ifndef __CAM_SYNC_API_H__ @@ -140,6 +140,16 @@ int cam_sync_destroy(int32_t sync_obj); */ int cam_sync_wait(int32_t sync_obj, uint64_t timeout_ms); +/** + * @brief: Check if sync object is valid + * + * @param sync_obj: int referencing the sync object to be checked + * + * @return 0 upon success, -EINVAL if sync object is in bad state or arguments + * are invalid + */ +int cam_sync_check_valid(int32_t sync_obj); + /** * @brief : API to register SYNC to platform framework. * @return struct platform_device pointer on on success, or ERR_PTR() on error.