From dcfb376d706d85a27a9cdbda43b4701747dafd6f Mon Sep 17 00:00:00 2001 From: Soumya Managoli Date: Wed, 5 Apr 2023 15:46:55 +0530 Subject: [PATCH 1/9] dsp: q6adm: Resolve mem corruption in adm cb Conversion of negative "num_modules" var value will result in max possible unsigned int value and hence can cause mem corruption when accessed. Resolve this by assigning the same data type to "num_modules" var as used in the calling fn. Change-Id: I4c9d7215b9c7345637e1eb3a1992a41fef71c5cb Signed-off-by: Soumya Managoli --- dsp/q6adm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsp/q6adm.c b/dsp/q6adm.c index f2ae327ee554..712ba088baeb 100644 --- a/dsp/q6adm.c +++ b/dsp/q6adm.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2012-2021, The Linux Foundation. All rights reserved. - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. */ #include #include @@ -1668,7 +1668,7 @@ static int32_t adm_callback(struct apr_client_data *data, void *priv) { uint32_t *payload; int port_idx, copp_idx, idx, client_id; - int num_modules; + uint32_t num_modules; int ret; int payload_size = 0, i = 0; struct msm_adsp_event_data *pp_event_package = NULL; From 3ebb66aa5dfe4f4c71d359a0035a1c600f43fc5a Mon Sep 17 00:00:00 2001 From: Soumya Managoli Date: Thu, 6 Apr 2023 13:55:23 +0530 Subject: [PATCH 2/9] ASoC: msm-pcm-host-voice: Handle OOB access in hpcm_start There is no error check for case when hpcm_start is called for the same RX or TX tap points multiple times. This can result in OOB access of struct vss_ivpcm_tap_point. Handle this scenario with appropriate no_of_tp check. Change-Id: Ib384d21c9bf372f3e5d78f64b5c056e836728399 Signed-off-by: Soumya Managoli --- asoc/msm-pcm-host-voice-v2.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/asoc/msm-pcm-host-voice-v2.c b/asoc/msm-pcm-host-voice-v2.c index 4496e05a966a..50fbd15d0481 100644 --- a/asoc/msm-pcm-host-voice-v2.c +++ b/asoc/msm-pcm-host-voice-v2.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright (c) 2013-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -626,6 +627,12 @@ static int hpcm_start_vocpcm(char *pcm_id, struct hpcm_drv *prtd, } } + if (*no_of_tp != no_of_tp_req && *no_of_tp > 2) { + pr_err("%s:: Invalid hpcm start request\n", __func__); + memset(&prtd->start_cmd, 0, sizeof(struct start_cmd)); + return -EINVAL; + } + if ((prtd->mixer_conf.tx.enable || prtd->mixer_conf.rx.enable) && *no_of_tp == no_of_tp_req) { voc_send_cvp_start_vocpcm(voc_get_session_id(sess_name), From 8c0d7cffabc96ab0bfe87df61f5b35b92cf9533e Mon Sep 17 00:00:00 2001 From: Soumya Managoli Date: Tue, 11 Apr 2023 10:17:55 +0530 Subject: [PATCH 3/9] dsp: q6voice: Add buf size check for cvs cal data Check for the max size of cvs command register calibration data that can be copied else will result in buffer overflow. Change-Id: Id7a4c5a9795143798b68dfde779f17fb450e3848 Signed-off-by: Soumya Managoli --- dsp/q6voice.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dsp/q6voice.c b/dsp/q6voice.c index 1f3b1448fb8d..0f2fe8ccc1b4 100644 --- a/dsp/q6voice.c +++ b/dsp/q6voice.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2012-2021, The Linux Foundation. All rights reserved. - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved. */ #include #include @@ -2852,6 +2852,13 @@ static int voice_send_cvs_register_cal_cmd(struct voice_data *v) goto unlock; } + if (col_data->cal_data.size >= MAX_COL_INFO_SIZE) { + pr_err("%s: Invalid cal data size %d!\n", + __func__, col_data->cal_data.size); + ret = -EINVAL; + goto unlock; + } + memcpy(&cvs_reg_cal_cmd.cvs_cal_data.column_info[0], (void *) &((struct audio_cal_info_voc_col *) col_data->cal_info)->data, From 3b0442cec3d57c0b8a8d44c4832f837013c23b2f Mon Sep 17 00:00:00 2001 From: Soumya Managoli Date: Wed, 12 Apr 2023 12:34:26 +0530 Subject: [PATCH 4/9] dsp: q6core: Avoid OOB access in q6core "num_services", a signed integer when compared with constant results in conversion of signed integer to max possible unsigned int value when "num_services" is a negative value. This can lead to OOB read. Fix is to handle this case. Change-Id: Id6a8f150d9019c972a87f789e4c626337a97bfff Signed-off-by: Soumya Managoli --- dsp/q6core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dsp/q6core.c b/dsp/q6core.c index 2fce87998485..35fc8a801261 100644 --- a/dsp/q6core.c +++ b/dsp/q6core.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -204,7 +205,7 @@ EXPORT_SYMBOL(q6core_send_uevent); static int parse_fwk_version_info(uint32_t *payload, uint16_t payload_size) { size_t ver_size; - int num_services; + uint16_t num_services; pr_debug("%s: Payload info num services %d\n", __func__, payload[4]); From 7756692d5419f47792b778f9cc4a43d468d0bee8 Mon Sep 17 00:00:00 2001 From: Soumya Managoli Date: Wed, 12 Apr 2023 14:27:28 +0530 Subject: [PATCH 5/9] ASoC: msm-pcm-host-voice: Address buffer overflow in hpcm capture copy There is no check for the copy data size of the ADSP returned payload. Add check for the max hpcm_buf_node size before copy to avoid buffer out of bounds issue. Change-Id: Id647888430ce302359a857ef54d321bee99889bf Signed-off-by: Soumya Managoli --- asoc/msm-pcm-host-voice-v2.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/asoc/msm-pcm-host-voice-v2.c b/asoc/msm-pcm-host-voice-v2.c index 4496e05a966a..b3100414af3a 100644 --- a/asoc/msm-pcm-host-voice-v2.c +++ b/asoc/msm-pcm-host-voice-v2.c @@ -683,6 +683,12 @@ static void hpcm_copy_capture_data_to_queue(struct dai_data *dai_data, if (dai_data->substream == NULL) return; + if (len >= HPCM_MAX_VOC_PKT_SIZE) { + pr_err("%s: Copy capture data len %d is > HPCM_MAX_VOC_PKT_SIZE\n", + __func__, len); + return; + } + /* Copy out buffer packet into free_queue */ spin_lock_irqsave(&dai_data->dsp_lock, dsp_flags); From 2f6eb70111c444d23639c0bf3b625bf6140b1443 Mon Sep 17 00:00:00 2001 From: Soumya Managoli Date: Mon, 17 Apr 2023 17:02:59 +0530 Subject: [PATCH 6/9] ASoC: msm-pcm-voip: Avoid interger underflow There is no check for voip pkt pkt_len,if it contains the min required data. This can lead to integer underflow. Add check for the same. Change-Id: I4f57eb125967d52ad8da60d21a440af1f81d2579 Signed-off-by: Soumya Managoli --- asoc/msm-pcm-voip-v2.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/asoc/msm-pcm-voip-v2.c b/asoc/msm-pcm-voip-v2.c index 14803f1d235a..c926fa4e3037 100644 --- a/asoc/msm-pcm-voip-v2.c +++ b/asoc/msm-pcm-voip-v2.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright (c) 2012-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -365,6 +366,13 @@ static void voip_process_ul_pkt(uint8_t *voc_pkt, switch (prtd->mode) { case MODE_AMR_WB: case MODE_AMR: { + if (pkt_len <= DSP_FRAME_HDR_LEN) { + pr_err("%s: pkt_len %d is < required len\n", + pkt_len); + spin_unlock_irqrestore(&prtd->dsp_ul_lock, + dsp_flags); + return; + } /* Remove the DSP frame info header. Header format: * Bits 0-3: Frame rate * Bits 4-7: Frame type @@ -385,6 +393,13 @@ static void voip_process_ul_pkt(uint8_t *voc_pkt, case MODE_4GV_NB: case MODE_4GV_WB: case MODE_4GV_NW: { + if (pkt_len <= DSP_FRAME_HDR_LEN) { + pr_err("%s: pkt_len %d is < required len\n", + pkt_len); + spin_unlock_irqrestore(&prtd->dsp_ul_lock, + dsp_flags); + return; + } /* Remove the DSP frame info header. * Header format: * Bits 0-3: frame rate @@ -422,6 +437,13 @@ static void voip_process_ul_pkt(uint8_t *voc_pkt, buf_node->frame.frm_hdr.timestamp = timestamp; voc_pkt = voc_pkt + DSP_FRAME_HDR_LEN; + if (pkt_len <= 2 * DSP_FRAME_HDR_LEN) { + pr_err("%s: pkt_len %d is < required len\n", + pkt_len); + spin_unlock_irqrestore(&prtd->dsp_ul_lock, + dsp_flags); + return; + } /* There are two frames in the buffer. Length of the * first frame: */ From 17335ad139121dc1f4bc7df934fb9c4a9a47d77c Mon Sep 17 00:00:00 2001 From: Shalini Manjunatha Date: Mon, 10 Apr 2023 16:19:35 +0530 Subject: [PATCH 7/9] dsp: afe: check for param size before copying check for the proper param size before copying, to avoid buffer overflow. Change-Id: I70c52e6ab76f528ea3714784ab9013b070839c40 Signed-off-by: Shalini Manjunatha --- dsp/q6afe.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/dsp/q6afe.c b/dsp/q6afe.c index a08843a74d31..d1acb5c9fe0e 100644 --- a/dsp/q6afe.c +++ b/dsp/q6afe.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright (c) 2012-2021, The Linux Foundation. All rights reserved. - * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved. */ #include #include @@ -786,32 +786,74 @@ static int32_t sp_make_afe_callback(uint32_t opcode, uint32_t *payload, switch (param_hdr.param_id) { case AFE_PARAM_ID_CALIB_RES_CFG_V2: expected_size += sizeof(struct asm_calib_res_cfg); + if (param_hdr.param_size != sizeof(struct asm_calib_res_cfg)) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } data_dest = (u32 *) &this_afe.calib_data; break; case AFE_PARAM_ID_SP_V2_TH_VI_FTM_PARAMS: expected_size += sizeof(struct afe_sp_th_vi_ftm_params); + if (param_hdr.param_size != sizeof(struct afe_sp_th_vi_ftm_params)) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } data_dest = (u32 *) &this_afe.th_vi_resp; break; case AFE_PARAM_ID_SP_V2_TH_VI_V_VALI_PARAMS: expected_size += sizeof(struct afe_sp_th_vi_v_vali_params); + if (param_hdr.param_size != sizeof(struct afe_sp_th_vi_v_vali_params)) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } data_dest = (u32 *) &this_afe.th_vi_v_vali_resp; break; case AFE_PARAM_ID_SP_V2_EX_VI_FTM_PARAMS: expected_size += sizeof(struct afe_sp_ex_vi_ftm_params); + if (param_hdr.param_size != sizeof(struct afe_sp_ex_vi_ftm_params)) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } data_dest = (u32 *) &this_afe.ex_vi_resp; break; case AFE_PARAM_ID_SP_RX_TMAX_XMAX_LOGGING: expected_size += sizeof( struct afe_sp_rx_tmax_xmax_logging_param); + if (param_hdr.param_size != sizeof(struct afe_sp_rx_tmax_xmax_logging_param)) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } data_dest = (u32 *) &this_afe.xt_logging_resp; break; case AFE_PARAM_ID_SP_V4_CALIB_RES_CFG: expected_size += sizeof( struct afe_sp_v4_param_th_vi_calib_res_cfg); + if (param_hdr.param_size != sizeof( + struct afe_sp_v4_param_th_vi_calib_res_cfg)) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } data_dest = (u32 *) &this_afe.spv4_calib_data; break; case AFE_PARAM_ID_SP_V4_TH_VI_FTM_PARAMS: num_ch = data_start[0]; + if (num_ch > SP_V2_NUM_MAX_SPKRS) { + pr_err("%s: Error: num_ch %d is greater than expected\n", + __func__,num_ch); + return -EINVAL; + } + if (param_hdr.param_size != (sizeof(struct afe_sp_v4_param_th_vi_ftm_params) + + (num_ch * sizeof(struct afe_sp_v4_channel_ftm_params)))) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } this_afe.spv4_th_vi_ftm_rcvd_param_size = param_hdr.param_size; data_dest = (u32 *)&this_afe.spv4_th_vi_ftm_resp; expected_size += @@ -820,6 +862,18 @@ static int32_t sp_make_afe_callback(uint32_t opcode, uint32_t *payload, break; case AFE_PARAM_ID_SP_V4_TH_VI_V_VALI_PARAMS: num_ch = data_start[0]; + if (num_ch > SP_V2_NUM_MAX_SPKRS) { + pr_err("%s: Error: num_ch %d is greater than expected\n", + __func__,num_ch); + return -EINVAL; + } + if (param_hdr.param_size != (sizeof(struct afe_sp_v4_param_th_vi_v_vali_params) + + (num_ch * + sizeof(struct afe_sp_v4_channel_v_vali_params)))) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } this_afe.spv4_v_vali_rcvd_param_size = param_hdr.param_size; data_dest = (u32 *)&this_afe.spv4_v_vali_resp; expected_size += @@ -829,6 +883,18 @@ static int32_t sp_make_afe_callback(uint32_t opcode, uint32_t *payload, break; case AFE_PARAM_ID_SP_V4_EX_VI_FTM_PARAMS: num_ch = data_start[0]; + if (num_ch > SP_V2_NUM_MAX_SPKRS) { + pr_err("%s: Error: num_ch %d is greater than expected\n", + __func__,num_ch); + return -EINVAL; + } + if (param_hdr.param_size != (sizeof(struct afe_sp_v4_param_ex_vi_ftm_params) + + (num_ch * + sizeof(struct afe_sp_v4_channel_ex_vi_ftm_params)))) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } this_afe.spv4_ex_vi_ftm_rcvd_param_size = param_hdr.param_size; data_dest = (u32 *)&this_afe.spv4_ex_vi_ftm_resp; expected_size += @@ -837,6 +903,18 @@ static int32_t sp_make_afe_callback(uint32_t opcode, uint32_t *payload, break; case AFE_PARAM_ID_SP_V4_RX_TMAX_XMAX_LOGGING: num_ch = data_start[0]; + if (num_ch > SP_V2_NUM_MAX_SPKRS) { + pr_err("%s: Error: num_ch %d is greater than expected\n", + __func__,num_ch); + return -EINVAL; + } + if (param_hdr.param_size != (sizeof(struct afe_sp_v4_param_tmax_xmax_logging) + + (num_ch * + sizeof(struct afe_sp_v4_channel_tmax_xmax_params)))) { + pr_err("%s: Error: param_size %d is greater than expected\n", + __func__,param_hdr.param_size); + return -EINVAL; + } this_afe.spv4_max_log_rcvd_param_size = param_hdr.param_size; data_dest = (u32 *)&this_afe.spv4_max_log_resp; expected_size += From 1d75d0dcb6f1afeab757e1b331179ff69b6838e9 Mon Sep 17 00:00:00 2001 From: Shalini Manjunatha Date: Mon, 17 Apr 2023 16:49:39 +0530 Subject: [PATCH 8/9] dsp: q6core: validate payload size before access for AVCS Payload size is not checked before payload access for AVCS. Check size to avoid out-of-boundary memory access. Change-Id: I6de3342617bd4f3fb8849ad2230dd57c07469372 Signed-off-by: Shalini Manjunatha --- dsp/q6core.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dsp/q6core.c b/dsp/q6core.c index 2fce87998485..441c15c7a717 100644 --- a/dsp/q6core.c +++ b/dsp/q6core.c @@ -474,6 +474,12 @@ static int32_t aprv2_core_fn_q(struct apr_client_data *data, void *priv) case AVCS_CMD_RSP_LOAD_MODULES: pr_debug("%s: Received AVCS_CMD_RSP_LOAD_MODULES\n", __func__); + if (data->payload_size != ((sizeof(struct avcs_load_unload_modules_sec_payload) + * rsp_payload->num_modules) + sizeof(uint32_t))) { + pr_err("%s: payload size greater than expected size %d\n", + __func__,data->payload_size); + return -EINVAL; + } memcpy(rsp_payload, data->payload, data->payload_size); q6core_lcl.avcs_module_resp_received = 1; wake_up(&q6core_lcl.avcs_module_load_unload_wait); @@ -1036,6 +1042,8 @@ int32_t q6core_avcs_load_unload_modules(struct avcs_load_unload_modules_payload return -ENOMEM; } + rsp_payload->num_modules = num_modules; + memcpy((uint8_t *)mod + sizeof(struct apr_hdr) + sizeof(struct avcs_load_unload_modules_meminfo), payload, payload_size); From c3b67277c613a8b4a809bf21d6fbf0f836ca384c Mon Sep 17 00:00:00 2001 From: Soumya Managoli Date: Mon, 29 May 2023 14:12:05 +0530 Subject: [PATCH 9/9] ASoC: msm-pcm-voip: Avoid interger underflow There is no check for voip pkt pkt_len,if it contains the min required data. This can lead to integer underflow. Add check for the same. Change-Id: I40242429542b6c32a0e6c3bbe03975c244c2f61a Signed-off-by: Soumya Managoli --- asoc/msm-pcm-voip-v2.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/asoc/msm-pcm-voip-v2.c b/asoc/msm-pcm-voip-v2.c index c926fa4e3037..cb71831fd22b 100644 --- a/asoc/msm-pcm-voip-v2.c +++ b/asoc/msm-pcm-voip-v2.c @@ -368,7 +368,7 @@ static void voip_process_ul_pkt(uint8_t *voc_pkt, case MODE_AMR: { if (pkt_len <= DSP_FRAME_HDR_LEN) { pr_err("%s: pkt_len %d is < required len\n", - pkt_len); + __func__, pkt_len); spin_unlock_irqrestore(&prtd->dsp_ul_lock, dsp_flags); return; @@ -395,7 +395,7 @@ static void voip_process_ul_pkt(uint8_t *voc_pkt, case MODE_4GV_NW: { if (pkt_len <= DSP_FRAME_HDR_LEN) { pr_err("%s: pkt_len %d is < required len\n", - pkt_len); + __func__, pkt_len); spin_unlock_irqrestore(&prtd->dsp_ul_lock, dsp_flags); return; @@ -439,7 +439,7 @@ static void voip_process_ul_pkt(uint8_t *voc_pkt, if (pkt_len <= 2 * DSP_FRAME_HDR_LEN) { pr_err("%s: pkt_len %d is < required len\n", - pkt_len); + __func__, pkt_len); spin_unlock_irqrestore(&prtd->dsp_ul_lock, dsp_flags); return; @@ -479,6 +479,14 @@ static void voip_process_ul_pkt(uint8_t *voc_pkt, buf_node->frame.frm_hdr.timestamp = timestamp; voc_pkt = voc_pkt + DSP_FRAME_HDR_LEN; + if (pkt_len <= 2 * DSP_FRAME_HDR_LEN) { + pr_err("%s: pkt_len %d is < required len\n", + __func__, pkt_len); + spin_unlock_irqrestore(&prtd->dsp_ul_lock, + dsp_flags); + return; + } + /* There are two frames in the buffer. Length * of the second frame: */