If dobj->control is not initialized we end up in an OOPs during
skl_tplg_complete:
[ 26.553358] BUG: kernel NULL pointer dereference, address:
0000000000000078
[ 26.561151] #PF: supervisor read access in kernel mode
[ 26.566897] #PF: error_code(0x0000) - not-present page
[ 26.572642] PGD 0 P4D 0
[ 26.575479] Oops: 0000 [#1] PREEMPT SMP PTI
[ 26.580158] CPU: 2 PID: 2082 Comm: udevd Tainted: G C
5.4.81 #4
[ 26.588232] Hardware name: HP Soraka/Soraka, BIOS
Google_Soraka.10431.106.0 12/03/2019
[ 26.597082] RIP: 0010:skl_tplg_complete+0x70/0x144 [snd_soc_skl]
Fixes: 2d744ecf2b98 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHL")
Signed-off-by: Ricardo Ribalda <[email protected]>
---
sound/soc/intel/skylake/skl-topology.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index ae466cd59292..1ef30ca45410 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -3619,15 +3619,16 @@ static void skl_tplg_complete(struct snd_soc_component *component)
list_for_each_entry(dobj, &component->dobj_list, list) {
struct snd_kcontrol *kcontrol = dobj->control.kcontrol;
- struct soc_enum *se =
- (struct soc_enum *)kcontrol->private_value;
- char **texts = dobj->control.dtexts;
+ struct soc_enum *se;
+ char **texts;
char chan_text[4];
- if (dobj->type != SND_SOC_DOBJ_ENUM ||
- dobj->control.kcontrol->put !=
- skl_tplg_multi_config_set_dmic)
+ if (dobj->type != SND_SOC_DOBJ_ENUM || !kcontrol ||
+ kcontrol->put != skl_tplg_multi_config_set_dmic)
continue;
+
+ se = (struct soc_enum *)kcontrol->private_value;
+ texts = dobj->control.dtexts;
sprintf(chan_text, "c%d", mach->mach_params.dmic_num);
for (i = 0; i < se->items; i++) {
--
2.30.0.296.g2bfb1c46d8-goog
Clear struct snd_ctl_elem_value before calling ->put() to avoid any data
leak.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
sound/soc/intel/skylake/skl-topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 1ef30ca45410..b824086203b9 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -3632,7 +3632,7 @@ static void skl_tplg_complete(struct snd_soc_component *component)
sprintf(chan_text, "c%d", mach->mach_params.dmic_num);
for (i = 0; i < se->items; i++) {
- struct snd_ctl_elem_value val;
+ struct snd_ctl_elem_value val = {};
if (strstr(texts[i], chan_text)) {
val.value.enumerated.item[0] = i;
--
2.30.0.296.g2bfb1c46d8-goog
Hi,
This is just a v2 from my patch from December with the ={}: in a second patch.
Best regards!
On Thu, Jan 21, 2021 at 6:16 PM Ricardo Ribalda <[email protected]> wrote:
>
> If dobj->control is not initialized we end up in an OOPs during
> skl_tplg_complete:
>
> [ 26.553358] BUG: kernel NULL pointer dereference, address:
> 0000000000000078
> [ 26.561151] #PF: supervisor read access in kernel mode
> [ 26.566897] #PF: error_code(0x0000) - not-present page
> [ 26.572642] PGD 0 P4D 0
> [ 26.575479] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 26.580158] CPU: 2 PID: 2082 Comm: udevd Tainted: G C
> 5.4.81 #4
> [ 26.588232] Hardware name: HP Soraka/Soraka, BIOS
> Google_Soraka.10431.106.0 12/03/2019
> [ 26.597082] RIP: 0010:skl_tplg_complete+0x70/0x144 [snd_soc_skl]
>
> Fixes: 2d744ecf2b98 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHL")
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> sound/soc/intel/skylake/skl-topology.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
> index ae466cd59292..1ef30ca45410 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -3619,15 +3619,16 @@ static void skl_tplg_complete(struct snd_soc_component *component)
>
> list_for_each_entry(dobj, &component->dobj_list, list) {
> struct snd_kcontrol *kcontrol = dobj->control.kcontrol;
> - struct soc_enum *se =
> - (struct soc_enum *)kcontrol->private_value;
> - char **texts = dobj->control.dtexts;
> + struct soc_enum *se;
> + char **texts;
> char chan_text[4];
>
> - if (dobj->type != SND_SOC_DOBJ_ENUM ||
> - dobj->control.kcontrol->put !=
> - skl_tplg_multi_config_set_dmic)
> + if (dobj->type != SND_SOC_DOBJ_ENUM || !kcontrol ||
> + kcontrol->put != skl_tplg_multi_config_set_dmic)
> continue;
> +
> + se = (struct soc_enum *)kcontrol->private_value;
> + texts = dobj->control.dtexts;
> sprintf(chan_text, "c%d", mach->mach_params.dmic_num);
>
> for (i = 0; i < se->items; i++) {
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
--
Ricardo Ribalda
On 2021-01-21 6:16 PM, Ricardo Ribalda wrote:
> If dobj->control is not initialized we end up in an OOPs during
> skl_tplg_complete:
>
> [ 26.553358] BUG: kernel NULL pointer dereference, address:
> 0000000000000078
> [ 26.561151] #PF: supervisor read access in kernel mode
> [ 26.566897] #PF: error_code(0x0000) - not-present page
> [ 26.572642] PGD 0 P4D 0
> [ 26.575479] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 26.580158] CPU: 2 PID: 2082 Comm: udevd Tainted: G C
> 5.4.81 #4
> [ 26.588232] Hardware name: HP Soraka/Soraka, BIOS
> Google_Soraka.10431.106.0 12/03/2019
> [ 26.597082] RIP: 0010:skl_tplg_complete+0x70/0x144 [snd_soc_skl]
>
> Fixes: 2d744ecf2b98 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHL")
> Signed-off-by: Ricardo Ribalda <[email protected]>
Thanks for the fix, Ricardo.
Reviewed-by: Cezary Rojewski <[email protected]>
On 2021-01-21 6:16 PM, Ricardo Ribalda wrote:
> Clear struct snd_ctl_elem_value before calling ->put() to avoid any data
> leak.
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
Reviewed-by: Cezary Rojewski <[email protected]>
Thanks,
Czarek
Thanks Ricardo - I have checked it on Eve/Google Pixelbook
Tested-by: Lukasz Majczak <[email protected]>
czw., 21 sty 2021 o 18:33 Rojewski, Cezary <[email protected]>
napisał(a):
>
> On 2021-01-21 6:16 PM, Ricardo Ribalda wrote:
> > If dobj->control is not initialized we end up in an OOPs during
> > skl_tplg_complete:
> >
> > [ 26.553358] BUG: kernel NULL pointer dereference, address:
> > 0000000000000078
> > [ 26.561151] #PF: supervisor read access in kernel mode
> > [ 26.566897] #PF: error_code(0x0000) - not-present page
> > [ 26.572642] PGD 0 P4D 0
> > [ 26.575479] Oops: 0000 [#1] PREEMPT SMP PTI
> > [ 26.580158] CPU: 2 PID: 2082 Comm: udevd Tainted: G C
> > 5.4.81 #4
> > [ 26.588232] Hardware name: HP Soraka/Soraka, BIOS
> > Google_Soraka.10431.106.0 12/03/2019
> > [ 26.597082] RIP: 0010:skl_tplg_complete+0x70/0x144 [snd_soc_skl]
> >
> > Fixes: 2d744ecf2b98 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHL")
> > Signed-off-by: Ricardo Ribalda <[email protected]>
>
> Thanks for the fix, Ricardo.
>
> Reviewed-by: Cezary Rojewski <[email protected]>
>
On Thu, Jan 21, 2021 at 06:16:43PM +0100, Ricardo Ribalda wrote:
> If dobj->control is not initialized we end up in an OOPs during
> skl_tplg_complete:
>
> [ 26.553358] BUG: kernel NULL pointer dereference, address:
> 0000000000000078
> [ 26.561151] #PF: supervisor read access in kernel mode
> [ 26.566897] #PF: error_code(0x0000) - not-present page
> [ 26.572642] PGD 0 P4D 0
> [ 26.575479] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 26.580158] CPU: 2 PID: 2082 Comm: udevd Tainted: G C
> 5.4.81 #4
> [ 26.588232] Hardware name: HP Soraka/Soraka, BIOS
> Google_Soraka.10431.106.0 12/03/2019
> [ 26.597082] RIP: 0010:skl_tplg_complete+0x70/0x144 [snd_soc_skl]
Reviewed-by: Andy Shevchenko <[email protected]>
> Fixes: 2d744ecf2b98 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHL")
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> sound/soc/intel/skylake/skl-topology.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
> index ae466cd59292..1ef30ca45410 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -3619,15 +3619,16 @@ static void skl_tplg_complete(struct snd_soc_component *component)
>
> list_for_each_entry(dobj, &component->dobj_list, list) {
> struct snd_kcontrol *kcontrol = dobj->control.kcontrol;
> - struct soc_enum *se =
> - (struct soc_enum *)kcontrol->private_value;
> - char **texts = dobj->control.dtexts;
> + struct soc_enum *se;
> + char **texts;
A nit-pick: Can we place this after below line?
> char chan_text[4];
>
> - if (dobj->type != SND_SOC_DOBJ_ENUM ||
> - dobj->control.kcontrol->put !=
> - skl_tplg_multi_config_set_dmic)
> + if (dobj->type != SND_SOC_DOBJ_ENUM || !kcontrol ||
> + kcontrol->put != skl_tplg_multi_config_set_dmic)
> continue;
> +
> + se = (struct soc_enum *)kcontrol->private_value;
> + texts = dobj->control.dtexts;
> sprintf(chan_text, "c%d", mach->mach_params.dmic_num);
>
> for (i = 0; i < se->items; i++) {
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
--
With Best Regards,
Andy Shevchenko
On Thu, Jan 21, 2021 at 06:16:44PM +0100, Ricardo Ribalda wrote:
> Clear struct snd_ctl_elem_value before calling ->put() to avoid any data
> leak.
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> sound/soc/intel/skylake/skl-topology.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
> index 1ef30ca45410..b824086203b9 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -3632,7 +3632,7 @@ static void skl_tplg_complete(struct snd_soc_component *component)
> sprintf(chan_text, "c%d", mach->mach_params.dmic_num);
>
> for (i = 0; i < se->items; i++) {
> - struct snd_ctl_elem_value val;
> + struct snd_ctl_elem_value val = {};
>
> if (strstr(texts[i], chan_text)) {
> val.value.enumerated.item[0] = i;
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
--
With Best Regards,
Andy Shevchenko
On Thu, 21 Jan 2021 18:16:43 +0100, Ricardo Ribalda wrote:
> If dobj->control is not initialized we end up in an OOPs during
> skl_tplg_complete:
>
> [ 26.553358] BUG: kernel NULL pointer dereference, address:
> 0000000000000078
> [ 26.561151] #PF: supervisor read access in kernel mode
> [ 26.566897] #PF: error_code(0x0000) - not-present page
> [ 26.572642] PGD 0 P4D 0
> [ 26.575479] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 26.580158] CPU: 2 PID: 2082 Comm: udevd Tainted: G C
> 5.4.81 #4
> [ 26.588232] Hardware name: HP Soraka/Soraka, BIOS
> Google_Soraka.10431.106.0 12/03/2019
> [ 26.597082] RIP: 0010:skl_tplg_complete+0x70/0x144 [snd_soc_skl]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: Intel: Skylake: skl-topology: Fix OOPs ib skl_tplg_complete
commit: c1c3ba1f78354a20222d291ed6fedd17b7a74fd7
[2/2] ASoC: Intel: Skylake: Zero snd_ctl_elem_value
commit: 1d8fe0648e118fd495a2cb393a34eb8d428e7808
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