From 179ab9d53631326db5a063e68ce119b21af40a3c Mon Sep 17 00:00:00 2001 From: Raghavendar rao l Date: Wed, 25 May 2022 19:50:45 +0530 Subject: [PATCH 1/5] msm: ipa3: handling MISRA issue Added some NULL checks to avoid memcpy with zero size, Updated checks for unsigned integer variables. Change-Id: Ia426da6b1af04fb5226ba9bbf22f0fce11e4dff0 Signed-off-by: Raghavendar rao l --- drivers/platform/msm/ipa/ipa_v3/ipa_hw_stats.c | 4 ++-- drivers/platform/msm/ipa/ipa_v3/ipa_intf.c | 6 +++--- drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c | 6 +----- drivers/platform/msm/ipa/ipa_v3/ipa_rt.c | 3 ++- drivers/platform/msm/ipa/ipa_v3/rmnet_ipa.c | 3 +-- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/drivers/platform/msm/ipa/ipa_v3/ipa_hw_stats.c b/drivers/platform/msm/ipa/ipa_v3/ipa_hw_stats.c index 3ae0f9976cf8..aa7c3afa8c1c 100644 --- a/drivers/platform/msm/ipa/ipa_v3/ipa_hw_stats.c +++ b/drivers/platform/msm/ipa/ipa_v3/ipa_hw_stats.c @@ -2311,7 +2311,7 @@ static ssize_t ipa_debugfs_enable_disable_drop_stats(struct file *file, is_pipe = true; } if (dbg_buff[i] == seprator) { - if (pipe_num >= 0 && pipe_num < ipa3_ctx->ipa_num_pipes + if (pipe_num < ipa3_ctx->ipa_num_pipes && ipa3_get_client_by_pipe(pipe_num) < IPA_CLIENT_MAX) { IPADBG("pipe number %u\n", pipe_num); @@ -2326,7 +2326,7 @@ static ssize_t ipa_debugfs_enable_disable_drop_stats(struct file *file, is_pipe = false; } } - if (is_pipe && pipe_num >= 0 && pipe_num < ipa3_ctx->ipa_num_pipes && + if (is_pipe && pipe_num < ipa3_ctx->ipa_num_pipes && ipa3_get_client_by_pipe(pipe_num) < IPA_CLIENT_MAX) { IPADBG("pipe number %u\n", pipe_num); if (enable_pipe) diff --git a/drivers/platform/msm/ipa/ipa_v3/ipa_intf.c b/drivers/platform/msm/ipa/ipa_v3/ipa_intf.c index 49417831e1a8..980bdc1f5aa7 100644 --- a/drivers/platform/msm/ipa/ipa_v3/ipa_intf.c +++ b/drivers/platform/msm/ipa/ipa_v3/ipa_intf.c @@ -82,19 +82,19 @@ int ipa3_register_intf_ext(const char *name, const struct ipa_tx_intf *tx, return -EINVAL; } - if (tx && tx->num_props > IPA_NUM_PROPS_MAX) { + if (tx && ((tx->num_props > IPA_NUM_PROPS_MAX) || (tx->num_props == 0))) { IPAERR_RL("invalid tx num_props=%d max=%d\n", tx->num_props, IPA_NUM_PROPS_MAX); return -EINVAL; } - if (rx && rx->num_props > IPA_NUM_PROPS_MAX) { + if (rx && ((rx->num_props > IPA_NUM_PROPS_MAX) || (rx->num_props == 0))) { IPAERR_RL("invalid rx num_props=%d max=%d\n", rx->num_props, IPA_NUM_PROPS_MAX); return -EINVAL; } - if (ext && ext->num_props > IPA_NUM_PROPS_MAX) { + if (ext && ((ext->num_props > IPA_NUM_PROPS_MAX) || (ext->num_props == 0))) { IPAERR_RL("invalid ext num_props=%d max=%d\n", ext->num_props, IPA_NUM_PROPS_MAX); return -EINVAL; diff --git a/drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c b/drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c index 41544e22d2e9..28cfaf4bcfd5 100644 --- a/drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c +++ b/drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c @@ -897,11 +897,7 @@ int ipa3_qmi_add_offload_request_send( return -EINVAL; } - /* check if the filter rules from IPACM is valid */ - if (req->filter_spec_ex2_list_len < 0) { - IPAWANERR("IPACM pass invalid num of rules\n"); - return -EINVAL; - } else if (req->filter_spec_ex2_list_len == 0) { + if (req->filter_spec_ex2_list_len == 0) { IPAWANDBG("IPACM pass zero rules to Q6\n"); } else { IPAWANDBG("IPACM pass %u rules to Q6\n", diff --git a/drivers/platform/msm/ipa/ipa_v3/ipa_rt.c b/drivers/platform/msm/ipa/ipa_v3/ipa_rt.c index 8c21090ffdab..79166258abae 100644 --- a/drivers/platform/msm/ipa/ipa_v3/ipa_rt.c +++ b/drivers/platform/msm/ipa/ipa_v3/ipa_rt.c @@ -1083,7 +1083,7 @@ static int __ipa_finish_rt_rule_add(struct ipa3_rt_entry *entry, u32 *rule_hdl, if (tbl->rule_cnt < IPA_RULE_CNT_MAX) tbl->rule_cnt++; else - return -EINVAL; + goto table_insert_failed; if (entry->hdr) entry->hdr->ref_cnt++; else if (entry->proc_ctx) @@ -1107,6 +1107,7 @@ ipa_insert_failed: else if (entry->proc_ctx) entry->proc_ctx->ref_cnt--; idr_remove(tbl->rule_ids, entry->rule_id); +table_insert_failed: list_del(&entry->link); kmem_cache_free(ipa3_ctx->rt_rule_cache, entry); return -EPERM; diff --git a/drivers/platform/msm/ipa/ipa_v3/rmnet_ipa.c b/drivers/platform/msm/ipa/ipa_v3/rmnet_ipa.c index 96623321bbe5..6d47eac263a0 100644 --- a/drivers/platform/msm/ipa/ipa_v3/rmnet_ipa.c +++ b/drivers/platform/msm/ipa/ipa_v3/rmnet_ipa.c @@ -3410,8 +3410,7 @@ int rmnet_ipa3_set_tether_client_pipe( return -EFAULT; } /* error checking if dl_dst_pipe_len valid or not*/ - if (data->dl_dst_pipe_len > QMI_IPA_MAX_PIPES_V01 || - data->dl_dst_pipe_len < 0) { + if (data->dl_dst_pipe_len > QMI_IPA_MAX_PIPES_V01) { IPAWANERR("DL dst pipes %d exceeding max %d\n", data->dl_dst_pipe_len, QMI_IPA_MAX_PIPES_V01); From 6a73b4eff077e4afece4b7c79f3d71da29f7eb9d Mon Sep 17 00:00:00 2001 From: Jagadeesh Ponduru Date: Fri, 10 Jun 2022 23:54:26 +0530 Subject: [PATCH 2/5] msm: ipahal: modify parameter from eq_bitfield[i] to i From IPAv4.5 versions IPA_TOS_EQ is considered as not supported and it made to 0XFF, instead IPA_IS_PURE_ACK is given support. So, when validating the bitfield equations of them, passing eq_bitfield[i] will throw shift overflow warning as it takes 0xFF. So, added change to ensure there are no overflow warnings. Change-Id: I84384723433706784a4ef21b88f33b036edb0a5d Signed-off-by: Jagadeesh Ponduru --- drivers/platform/msm/ipa/ipa_v3/ipahal/ipahal_fltrt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/platform/msm/ipa/ipa_v3/ipahal/ipahal_fltrt.c b/drivers/platform/msm/ipa/ipa_v3/ipahal/ipahal_fltrt.c index d62f191f66d8..5baf8df008e3 100644 --- a/drivers/platform/msm/ipa/ipa_v3/ipahal/ipahal_fltrt.c +++ b/drivers/platform/msm/ipa/ipa_v3/ipahal/ipahal_fltrt.c @@ -3532,12 +3532,12 @@ int ipahal_fltrt_init(enum ipa_hw_type ipa_hw_type) if (!IPA_IS_RULE_EQ_VALID(i)) continue; - if (eq_bits & IPA_GET_RULE_EQ_BIT_PTRN(eq_bitfield[i])) { + if (eq_bits & IPA_GET_RULE_EQ_BIT_PTRN(i)) { IPAHAL_ERR("more than eq with same bit. eq=%d\n", i); WARN_ON(1); return -EFAULT; } - eq_bits |= IPA_GET_RULE_EQ_BIT_PTRN(eq_bitfield[i]); + eq_bits |= IPA_GET_RULE_EQ_BIT_PTRN(i); } mem = &ipahal_ctx->empty_fltrt_tbl; From f3f4d78a114b176db68e6de0ec9f6af146e263bc Mon Sep 17 00:00:00 2001 From: Prashanth Reddy Baddam Date: Thu, 9 Jun 2022 11:53:09 +0530 Subject: [PATCH 3/5] msm: ipa: fix to NULL terminate the pointer Fix to NULL terminate the peers list ptr after freeing it, to get rid of use after free issue. Change-Id: Ide9fde9e7648a7af561a5b0ae0fa085810e59ea6 Signed-off-by: Prashanth Reddy Baddam --- drivers/platform/msm/ipa/ipa_rm_peers_list.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/platform/msm/ipa/ipa_rm_peers_list.c b/drivers/platform/msm/ipa/ipa_rm_peers_list.c index b6046fd5146e..40ac927c42a6 100644 --- a/drivers/platform/msm/ipa/ipa_rm_peers_list.c +++ b/drivers/platform/msm/ipa/ipa_rm_peers_list.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2013-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved */ #include @@ -68,6 +69,7 @@ int ipa_rm_peers_list_create(int max_peers, list_alloc_fail: kfree(*peers_list); + *peers_list = NULL; bail: return result; } From f9a39ef50fbbd585440592d4144af00519ca833a Mon Sep 17 00:00:00 2001 From: Ashok Vuyyuru Date: Mon, 13 Jun 2022 22:47:58 +0530 Subject: [PATCH 4/5] msm: ipa3: Added changes to check QMI pointer valid or not After passing ipa_q6_clnt pointer to function, if it freed in different thread it may lead to NULL pointer access. So adding check to see passed pointer valid or not. Change-Id: I22e272ebdecc62756ee140081524ab4efdd3d02a Signed-off-by: Ashok Vuyyuru --- drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c b/drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c index 28cfaf4bcfd5..ad128a512606 100644 --- a/drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c +++ b/drivers/platform/msm/ipa/ipa_v3/ipa_qmi_service.c @@ -469,8 +469,8 @@ static int ipa3_qmi_send_req_wait(struct qmi_handle *client_handle, mutex_lock(&ipa3_qmi_lock); - if (!client_handle) { - + if (client_handle != ipa_q6_clnt) { + IPADBG("Q6 QMI clinet pointer already freed\n"); mutex_unlock(&ipa3_qmi_lock); return -EINVAL; } @@ -1857,6 +1857,7 @@ void ipa3_qmi_service_exit(void) workqueues_stopped = true; + IPADBG("Entry\n"); /* qmi-service */ if (ipa3_svc_handle != NULL) { qmi_handle_release(ipa3_svc_handle); @@ -1889,6 +1890,7 @@ void ipa3_qmi_service_exit(void) ipa3_qmi_indication_fin = false; ipa3_modem_init_cmplt = false; send_qmi_init_q6 = true; + IPADBG("Exit\n"); } void ipa3_qmi_stop_workqueues(void) From f0f97b40058fecd549fee8b27ee58a162b2ba097 Mon Sep 17 00:00:00 2001 From: Jagadeesh Ponduru Date: Tue, 21 Jun 2022 17:51:59 +0530 Subject: [PATCH 5/5] msm: ipa3: add check in odl pipe cleanup Adding a change to check whether the pipe has been setup or not before going to teardown the pipe. Change-Id: Ibb408fdf3509facd6c18e3238bdbfd7da0b4a643 Signed-off-by: Jagadeesh Ponduru --- drivers/platform/msm/ipa/ipa_v3/ipa_odl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/platform/msm/ipa/ipa_v3/ipa_odl.c b/drivers/platform/msm/ipa/ipa_v3/ipa_odl.c index ba2686809ebe..9d5b68d0cfa1 100644 --- a/drivers/platform/msm/ipa/ipa_v3/ipa_odl.c +++ b/drivers/platform/msm/ipa/ipa_v3/ipa_odl.c @@ -473,6 +473,12 @@ void ipa3_odl_pipe_cleanup(bool is_ssr) IPAERR("adpl pipe not configured\n"); return; } + + if(!ipa3_odl_ctx->odl_state.odl_ep_setup) { + IPAERR("adpl pipe setup not done\n"); + return; + } + if (ipa3_odl_ctx->odl_state.odl_open) ipa_odl_opened = true;