Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bb67f4b0-e96a-3718-f855-80e1d21e9d38@linaro.org>
Date:   Tue, 9 May 2023 13:26:32 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] ASoC:codecs: lpass: Fix for KASAN use_after_free out of
 bounds
Content-Language: en-US
To:     Ravulapati Vishnu Vardhan Rao <quic_visr@quicinc.com>
Cc:     Banajit Goswami <bgoswami@quicinc.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>,
        "moderated list:QCOM AUDIO (ASoC) DRIVERS" 
        <alsa-devel@alsa-project.org>,
        open list <linux-kernel@vger.kernel.org>
References: <20230509103232.20953-1-quic_visr@quicinc.com>
From:   Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
In-Reply-To: <20230509103232.20953-1-quic_visr@quicinc.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 09/05/2023 11:32, Ravulapati Vishnu Vardhan Rao wrote:
> When we run syzkaller we get below Out of Bounds error.
> 
> "KASAN: slab-out-of-bounds Read in regcache_flat_read"
> 
> Below is the backtrace of the issue:
> 
> BUG: KASAN: slab-out-of-bounds in regcache_flat_read+0x10c/0x110
> Read of size 4 at addr ffffff8088fbf714 by task syz-executor.4/14144
> CPU: 6 PID: 14144 Comm: syz-executor.4 Tainted: G        W
> Hardware name: Qualcomm Technologies, Inc. sc7280 CRD platform (rev5+) (DT)
> Call trace:
> dump_backtrace+0x0/0x4ec
> show_stack+0x34/0x50
> dump_stack_lvl+0xdc/0x11c
> print_address_description+0x30/0x2d8
> kasan_report+0x178/0x1e4
> __asan_report_load4_noabort+0x44/0x50
> regcache_flat_read+0x10c/0x110
> regcache_read+0xf8/0x5a0
> _regmap_read+0x45c/0x86c
> _regmap_update_bits+0x128/0x290
> regmap_update_bits_base+0xc0/0x15c
> snd_soc_component_update_bits+0xa8/0x22c
> snd_soc_component_write_field+0x68/0xd4
> tx_macro_put_dec_enum+0x1d0/0x268
> snd_ctl_elem_write+0x288/0x474
> 
> By Error checking and checking valid values issue gets rectifies.
> 
> Signed-off-by: Ravulapati Vishnu Vardhan Rao <quic_visr@quicinc.com>
> ---
>   sound/soc/codecs/lpass-tx-macro.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
> index da6fcf7f0991..2fc150b17f29 100644
> --- a/sound/soc/codecs/lpass-tx-macro.c
> +++ b/sound/soc/codecs/lpass-tx-macro.c
> @@ -746,6 +746,8 @@ static int tx_macro_put_dec_enum(struct snd_kcontrol *kcontrol,
>   	struct tx_macro *tx = snd_soc_component_get_drvdata(component);
>   
>   	val = ucontrol->value.enumerated.item[0];
> +	if (val < 0 && val > 15)
> +		return -EINVAL;

how about

if (val >= e->items)
	return -EINVAL;


We could get these checks if CONFIG_SND_CTL_INTPUT_VALIDATION  was enabled.

>   
>   	switch (e->reg) {
>   	case CDC_TX_INP_MUX_ADC_MUX0_CFG0:
> @@ -772,6 +774,9 @@ static int tx_macro_put_dec_enum(struct snd_kcontrol *kcontrol,
>   	case CDC_TX_INP_MUX_ADC_MUX7_CFG0:
>   		mic_sel_reg = CDC_TX7_TX_PATH_CFG0;
>   		break;
> +	default:
> +		dev_err(component->dev, "Error in configuration!!\n");
> +		return -EINVAL;
>   	}
>   
>   	if (val != 0) {
> @@ -785,10 +790,16 @@ static int tx_macro_put_dec_enum(struct snd_kcontrol *kcontrol,
>   			snd_soc_component_write_field(component, mic_sel_reg,
>   						      CDC_TXn_ADC_DMIC_SEL_MASK, 1);
>   			dmic = TX_ADC_TO_DMIC(val);
> -			dmic_clk_reg = CDC_TX_TOP_CSR_SWR_DMICn_CTL(dmic);
> -			snd_soc_component_write_field(component, dmic_clk_reg,
> -						CDC_TX_SWR_DMIC_CLK_SEL_MASK,
> -						tx->dmic_clk_div);
> +			if (dmic < 4) {
> +				dmic_clk_reg = CDC_TX_TOP_CSR_SWR_DMICn_CTL(dmic);
> +				snd_soc_component_write_field(component, dmic_clk_reg,
> +							      CDC_TX_SWR_DMIC_CLK_SEL_MASK,
> +								tx->dmic_clk_div);
> +			} else {
> +				dev_err(component->dev, "Error in dmic configuration!!\n");
> +				return -EINVAL;
> +			}

These changes look unrelated.

--srini

> +
>   		}
>   	}
>