2022-02-22 20:30:15

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH v2 0/9] ASoC: codecs: qcom fix validation failures

Thanks for pointing out to mixer kselftest and VALIDATION Kconfig.

This patchset addresses some of the issues in Qualcomm codecs that are
discovered with recent mixer kselftest and validations added to ASoC.

Mostly these are under
1. accessing integer value type for enum controls, which is clearly an array out of bounds access.
2. Fix incorrect ranges.
3. Fix return values for put functions.

Changes since v1:
- removed tlv min max patches as "9bdd10d57a ("ASoC: ops:
Shift tested values in snd_soc_put_volsw() by +min")" fixed the issue in core.
- added checks in wcd934x_rx_hph_mode_put before returning

thanks,
srini

Srinivas Kandagatla (9):
ASoC: codecs: rx-macro: fix accessing compander for aux
ASoC: codecs: rx-macro: fix accessing array out of bounds for enum
type
ASoC: codecs: tx-macro: fix accessing array out of bounds for enum
type
ASoC: codecs: va-macro: fix accessing array out of bounds for enum
type
ASoC: codecs: wsa-macro: fix accessing array out of bounds for enum
type
ASoC: codecs: wc938x: fix accessing array out of bounds for enum type
ASoC: codecs: wcd938x: fix kcontrol max values
ASoC: codecs: wcd934x: fix kcontrol max values
ASoC: codecs: wcd934x: fix return value of wcd934x_rx_hph_mode_put

sound/soc/codecs/lpass-rx-macro.c | 12 ++++++++----
sound/soc/codecs/lpass-tx-macro.c | 8 ++++----
sound/soc/codecs/lpass-va-macro.c | 4 ++--
sound/soc/codecs/lpass-wsa-macro.c | 10 +++++-----
sound/soc/codecs/wcd934x.c | 11 +++++++----
sound/soc/codecs/wcd938x.c | 10 +++++-----
6 files changed, 31 insertions(+), 24 deletions(-)

--
2.21.0


2022-02-22 20:30:51

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH v2 2/9] ASoC: codecs: rx-macro: fix accessing array out of bounds for enum type

Accessing enums using integer would result in array out of bounds access
on platforms like aarch64 where sizeof(long) is 8 compared to enum size
which is 4 bytes.

Fixes: 4f692926f562 ("ASoC: codecs: lpass-rx-macro: add dapm widgets and route")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-rx-macro.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
index 32e85d2e9b90..3a3dc0539d92 100644
--- a/sound/soc/codecs/lpass-rx-macro.c
+++ b/sound/soc/codecs/lpass-rx-macro.c
@@ -2272,7 +2272,7 @@ static int rx_macro_mux_get(struct snd_kcontrol *kcontrol,
struct snd_soc_component *component = snd_soc_dapm_to_component(widget->dapm);
struct rx_macro *rx = snd_soc_component_get_drvdata(component);

- ucontrol->value.integer.value[0] =
+ ucontrol->value.enumerated.item[0] =
rx->rx_port_value[widget->shift];
return 0;
}
@@ -2284,7 +2284,7 @@ static int rx_macro_mux_put(struct snd_kcontrol *kcontrol,
struct snd_soc_component *component = snd_soc_dapm_to_component(widget->dapm);
struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
struct snd_soc_dapm_update *update = NULL;
- u32 rx_port_value = ucontrol->value.integer.value[0];
+ u32 rx_port_value = ucontrol->value.enumerated.item[0];
u32 aif_rst;
struct rx_macro *rx = snd_soc_component_get_drvdata(component);

@@ -2396,7 +2396,7 @@ static int rx_macro_get_hph_pwr_mode(struct snd_kcontrol *kcontrol,
struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
struct rx_macro *rx = snd_soc_component_get_drvdata(component);

- ucontrol->value.integer.value[0] = rx->hph_pwr_mode;
+ ucontrol->value.enumerated.item[0] = rx->hph_pwr_mode;
return 0;
}

@@ -2406,7 +2406,7 @@ static int rx_macro_put_hph_pwr_mode(struct snd_kcontrol *kcontrol,
struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
struct rx_macro *rx = snd_soc_component_get_drvdata(component);

- rx->hph_pwr_mode = ucontrol->value.integer.value[0];
+ rx->hph_pwr_mode = ucontrol->value.enumerated.item[0];
return 0;
}

--
2.21.0

2022-02-22 20:53:41

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH v2 7/9] ASoC: codecs: wcd938x: fix kcontrol max values

set "HPH Type" Kcontrol max value of WCD_MBHC_HPH_STEREO instead of UINT_MAX.
set "HPHL/R Impedance" Kcontrols max value to INT_MAX instead of UINT_MAX as
max field is integer type.

Without this patch amixer for these controls will show -1 as max value to userspace.

Fixes: bcee7ed09b8e ("ASoC: codecs: wcd938x: add Multi Button Headset Control support")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/wcd938x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
index deecc83eb592..782877db8c3c 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -3577,14 +3577,14 @@ static int wcd938x_hph_impedance_get(struct snd_kcontrol *kcontrol,
}

static const struct snd_kcontrol_new hph_type_detect_controls[] = {
- SOC_SINGLE_EXT("HPH Type", 0, 0, UINT_MAX, 0,
+ SOC_SINGLE_EXT("HPH Type", 0, 0, WCD_MBHC_HPH_STEREO, 0,
wcd938x_get_hph_type, NULL),
};

static const struct snd_kcontrol_new impedance_detect_controls[] = {
- SOC_SINGLE_EXT("HPHL Impedance", 0, 0, UINT_MAX, 0,
+ SOC_SINGLE_EXT("HPHL Impedance", 0, 0, INT_MAX, 0,
wcd938x_hph_impedance_get, NULL),
- SOC_SINGLE_EXT("HPHR Impedance", 0, 1, UINT_MAX, 0,
+ SOC_SINGLE_EXT("HPHR Impedance", 0, 1, INT_MAX, 0,
wcd938x_hph_impedance_get, NULL),
};

--
2.21.0

2022-02-22 22:03:12

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH v2 4/9] ASoC: codecs: va-macro: fix accessing array out of bounds for enum type

Accessing enums using integer would result in array out of bounds access
on platforms like aarch64 where sizeof(long) is 8 compared to enum size
which is 4 bytes.

Fixes: 908e6b1df26e ("ASoC: codecs: lpass-va-macro: Add support to VA Macro")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-va-macro.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
index 11147e35689b..e14c277e6a8b 100644
--- a/sound/soc/codecs/lpass-va-macro.c
+++ b/sound/soc/codecs/lpass-va-macro.c
@@ -780,7 +780,7 @@ static int va_macro_dec_mode_get(struct snd_kcontrol *kcontrol,
struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
int path = e->shift_l;

- ucontrol->value.integer.value[0] = va->dec_mode[path];
+ ucontrol->value.enumerated.item[0] = va->dec_mode[path];

return 0;
}
@@ -789,7 +789,7 @@ static int va_macro_dec_mode_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
{
struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
- int value = ucontrol->value.integer.value[0];
+ int value = ucontrol->value.enumerated.item[0];
struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
int path = e->shift_l;
struct va_macro *va = snd_soc_component_get_drvdata(comp);
--
2.21.0

2022-02-23 01:44:34

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH v2 1/9] ASoC: codecs: rx-macro: fix accessing compander for aux

AUX interpolator does not have compander, so check before accessing
compander data for this.

Without this checkan array of out bounds access will be made in
comp_enabled[] array.

Fixes: 4f692926f562 ("ASoC: codecs: lpass-rx-macro: add dapm widgets and route")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-rx-macro.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
index 2b272a82eabf..32e85d2e9b90 100644
--- a/sound/soc/codecs/lpass-rx-macro.c
+++ b/sound/soc/codecs/lpass-rx-macro.c
@@ -2039,6 +2039,10 @@ static int rx_macro_load_compander_coeff(struct snd_soc_component *component,
int i;
int hph_pwr_mode;

+ /* AUX does not have compander */
+ if (comp == INTERP_AUX)
+ return 0;
+
if (!rx->comp_enabled[comp])
return 0;

--
2.21.0

2022-02-23 01:52:17

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH v2 3/9] ASoC: codecs: tx-macro: fix accessing array out of bounds for enum type

Accessing enums using integer would result in array out of bounds access
on platforms like aarch64 where sizeof(long) is 8 compared to enum size
which is 4 bytes.

Also few return value of put functions, so that change notifications are
sent correctly.

Fixes: c39667ddcfc5 ("ASoC: codecs: lpass-tx-macro: add support for lpass tx macro")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-tx-macro.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
index 9c96ab1bf84f..7347d79a6329 100644
--- a/sound/soc/codecs/lpass-tx-macro.c
+++ b/sound/soc/codecs/lpass-tx-macro.c
@@ -997,7 +997,7 @@ static int tx_macro_dec_mode_get(struct snd_kcontrol *kcontrol,
struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
int path = e->shift_l;

- ucontrol->value.integer.value[0] = tx->dec_mode[path];
+ ucontrol->value.enumerated.item[0] = tx->dec_mode[path];

return 0;
}
@@ -1006,14 +1006,14 @@ static int tx_macro_dec_mode_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
{
struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
- int value = ucontrol->value.integer.value[0];
+ int value = ucontrol->value.enumerated.item[0];
struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
int path = e->shift_l;
struct tx_macro *tx = snd_soc_component_get_drvdata(component);

tx->dec_mode[path] = value;

- return 0;
+ return 1;
}

static int tx_macro_get_bcs(struct snd_kcontrol *kcontrol,
@@ -1036,7 +1036,7 @@ static int tx_macro_set_bcs(struct snd_kcontrol *kcontrol,

tx->bcs_enable = value;

- return 0;
+ return 1;
}

static int tx_macro_hw_params(struct snd_pcm_substream *substream,
--
2.21.0

2022-02-23 02:29:53

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH v2 5/9] ASoC: codecs: wsa-macro: fix accessing array out of bounds for enum type

Accessing enums using integer would result in array out of bounds access
on platforms like aarch64 where sizeof(long) is 8 compared to enum size
which is 4 bytes.

Also fix return value of put function, so that change notifications are
sent correctly.

Fixes: 2c4066e5d428 ("ASoC: codecs: lpass-wsa-macro: add dapm widgets and route")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-wsa-macro.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index 69d2915f40d8..ddde17e2dc35 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -1805,7 +1805,7 @@ static int wsa_macro_ear_spkr_pa_gain_get(struct snd_kcontrol *kcontrol,
struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);

- ucontrol->value.integer.value[0] = wsa->ear_spkr_gain;
+ ucontrol->value.enumerated.item[0] = wsa->ear_spkr_gain;

return 0;
}
@@ -1816,7 +1816,7 @@ static int wsa_macro_ear_spkr_pa_gain_put(struct snd_kcontrol *kcontrol,
struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);

- wsa->ear_spkr_gain = ucontrol->value.integer.value[0];
+ wsa->ear_spkr_gain = ucontrol->value.enumerated.item[0];

return 0;
}
@@ -1830,7 +1830,7 @@ static int wsa_macro_rx_mux_get(struct snd_kcontrol *kcontrol,
snd_soc_dapm_to_component(widget->dapm);
struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);

- ucontrol->value.integer.value[0] =
+ ucontrol->value.enumerated.item[0] =
wsa->rx_port_value[widget->shift];
return 0;
}
@@ -1844,7 +1844,7 @@ static int wsa_macro_rx_mux_put(struct snd_kcontrol *kcontrol,
snd_soc_dapm_to_component(widget->dapm);
struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
struct snd_soc_dapm_update *update = NULL;
- u32 rx_port_value = ucontrol->value.integer.value[0];
+ u32 rx_port_value = ucontrol->value.enumerated.item[0];
u32 bit_input;
u32 aif_rst;
struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
@@ -1887,7 +1887,7 @@ static int wsa_macro_rx_mux_put(struct snd_kcontrol *kcontrol,

snd_soc_dapm_mux_update_power(widget->dapm, kcontrol,
rx_port_value, e, update);
- return 0;
+ return 1;
}

static int wsa_macro_soft_clip_enable_get(struct snd_kcontrol *kcontrol,
--
2.21.0

2022-02-23 02:30:29

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH v2 9/9] ASoC: codecs: wcd934x: fix return value of wcd934x_rx_hph_mode_put

wcd934x_rx_hph_mode_put currently returns zero eventhough it changes the value.
Fix this, so that change notifications are sent correctly.

Fixes: 1cde8b822332 ("ASoC: wcd934x: add basic controls")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/wcd934x.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c
index f2674905a4a7..40b414867872 100644
--- a/sound/soc/codecs/wcd934x.c
+++ b/sound/soc/codecs/wcd934x.c
@@ -3308,13 +3308,16 @@ static int wcd934x_rx_hph_mode_put(struct snd_kcontrol *kc,

mode_val = ucontrol->value.enumerated.item[0];

+ if (mode_val == wcd->hph_mode)
+ return 0;
+
if (mode_val == 0) {
dev_err(wcd->dev, "Invalid HPH Mode, default to ClSH HiFi\n");
mode_val = CLS_H_LOHIFI;
}
wcd->hph_mode = mode_val;

- return 0;
+ return 1;
}

static int slim_rx_mux_get(struct snd_kcontrol *kc,
--
2.21.0

2022-02-23 02:40:31

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 3/9] ASoC: codecs: tx-macro: fix accessing array out of bounds for enum type

On Tue, Feb 22, 2022 at 06:32:06PM +0000, Srinivas Kandagatla wrote:

> Also few return value of put functions, so that change notifications are
> sent correctly.

The unconditional change notifications are present in this patch and
I'm guessing some others as well (they were there in a bunch
previously):

> tx->dec_mode[path] = value;
>
> - return 0;
> + return 1;
> }


Attachments:
(No filename) (391.00 B)
signature.asc (499.00 B)
Download all attachments

2022-02-23 09:10:08

by Mark Brown

[permalink] [raw]
Subject: Re: (subset) [PATCH v2 0/9] ASoC: codecs: qcom fix validation failures

On Tue, 22 Feb 2022 18:32:03 +0000, Srinivas Kandagatla wrote:
> Thanks for pointing out to mixer kselftest and VALIDATION Kconfig.
>
> This patchset addresses some of the issues in Qualcomm codecs that are
> discovered with recent mixer kselftest and validations added to ASoC.
>
> Mostly these are under
> 1. accessing integer value type for enum controls, which is clearly an array out of bounds access.
> 2. Fix incorrect ranges.
> 3. Fix return values for put functions.
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-linus

Thanks!

[1/9] ASoC: codecs: rx-macro: fix accessing compander for aux
commit: 42c709c4e1ce4c136891530646c9abd5dff3524f
[2/9] ASoC: codecs: rx-macro: fix accessing array out of bounds for enum type
commit: bcfe5f76cc4051ea3f9eb5d2c8ea621641f290a5
[4/9] ASoC: codecs: va-macro: fix accessing array out of bounds for enum type
commit: 0ea5eff7c6063a8f124188424f8e4c6727f35051
[6/9] ASoC: codecs: wc938x: fix accessing array out of bounds for enum type
commit: cc587b7c8fbbe128f6bd0dad025a0caea5e6d164
[7/9] ASoC: codecs: wcd938x: fix kcontrol max values
commit: b0217519236924f77a8382b4004e43ef8fd0dcbb
[8/9] ASoC: codecs: wcd934x: fix kcontrol max values
commit: 61163c3e7480106804269182e24db05244866493
[9/9] ASoC: codecs: wcd934x: fix return value of wcd934x_rx_hph_mode_put
commit: 4b0bec6088588a120d33db85b1f0d9f096d1df71

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

2022-02-23 23:31:24

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH v2 6/9] ASoC: codecs: wc938x: fix accessing array out of bounds for enum type

Accessing enums using integer would result in array out of bounds access
on platforms like aarch64 where sizeof(long) is 8 compared to enum size
which is 4 bytes.

Fix this by using enumerated items instead of integers.

Fixes: e8ba1e05bdc0 ("ASoC: codecs: wcd938x: add basic controls")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/wcd938x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
index cf61d23bfb1c..deecc83eb592 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -2506,7 +2506,7 @@ static int wcd938x_tx_mode_get(struct snd_kcontrol *kcontrol,
struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
int path = e->shift_l;

- ucontrol->value.integer.value[0] = wcd938x->tx_mode[path];
+ ucontrol->value.enumerated.item[0] = wcd938x->tx_mode[path];

return 0;
}
@@ -2530,7 +2530,7 @@ static int wcd938x_rx_hph_mode_get(struct snd_kcontrol *kcontrol,
struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);

- ucontrol->value.integer.value[0] = wcd938x->hph_mode;
+ ucontrol->value.enumerated.item[0] = wcd938x->hph_mode;

return 0;
}
--
2.21.0