[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(-)
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 srinivas.kandagatla@linaro.org --- 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;
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 srinivas.kandagatla@linaro.org --- 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; }
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 srinivas.kandagatla@linaro.org --- 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,
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;
}
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 srinivas.kandagatla@linaro.org --- 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);
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 srinivas.kandagatla@linaro.org --- 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,
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 srinivas.kandagatla@linaro.org --- 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; }
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 srinivas.kandagatla@linaro.org --- 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), };
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: 9fb9b1690f0b ("ASoC: codecs: wcd934x: add mbhc support") Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- sound/soc/codecs/wcd934x.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c index 6c468527fec6..f2674905a4a7 100644 --- a/sound/soc/codecs/wcd934x.c +++ b/sound/soc/codecs/wcd934x.c @@ -3023,14 +3023,14 @@ static int wcd934x_hph_impedance_get(struct snd_kcontrol *kcontrol, return 0; } 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, wcd934x_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, wcd934x_hph_impedance_get, NULL), - SOC_SINGLE_EXT("HPHR Impedance", 0, 1, UINT_MAX, 0, + SOC_SINGLE_EXT("HPHR Impedance", 0, 1, INT_MAX, 0, wcd934x_hph_impedance_get, NULL), };
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 srinivas.kandagatla@linaro.org --- 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,
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
- accessing integer value type for enum controls, which is clearly an array out of bounds access.
- Fix incorrect ranges.
- 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
participants (2)
-
Mark Brown -
Srinivas Kandagatla