There is no check for the kcontrol against NULL and in some cases
it causes kernel to crash.
Fixes: 2d744ecf2b984 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHLT")
Cc: <[email protected]> # 5.4+
Signed-off-by: Lukasz Majczak <[email protected]>
---
sound/soc/intel/skylake/skl-topology.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index ae466cd592922..c9abbe4ff0ba3 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -3618,12 +3618,18 @@ static void skl_tplg_complete(struct snd_soc_component *component)
int i;
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 snd_kcontrol *kcontrol;
+ struct soc_enum *se;
+ char **texts;
char chan_text[4];
+ kcontrol = dobj->control.kcontrol;
+ if(!kcontrol)
+ continue;
+
+ se = (struct soc_enum *)kcontrol->private_value;
+ texts = dobj->control.dtexts;
+
if (dobj->type != SND_SOC_DOBJ_ENUM ||
dobj->control.kcontrol->put !=
skl_tplg_multi_config_set_dmic)
base-commit: 69fe63aa100220c8fd1f451dd54dd0895df1441d
--
2.25.1
> There is no check for the kcontrol against NULL and in some cases
> it causes kernel to crash.
>
> Fixes: 2d744ecf2b984 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHLT")
> Cc: <[email protected]> # 5.4+
> Signed-off-by: Lukasz Majczak <[email protected]>
> ---
> sound/soc/intel/skylake/skl-topology.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
> index ae466cd592922..c9abbe4ff0ba3 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -3618,12 +3618,18 @@ static void skl_tplg_complete(struct snd_soc_component *component)
> int i;
>
> 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 snd_kcontrol *kcontrol;
> + struct soc_enum *se;
> + char **texts;
> char chan_text[4];
>
> + kcontrol = dobj->control.kcontrol;
> + if(!kcontrol)
> + continue;
> +
> + se = (struct soc_enum *)kcontrol->private_value;
> + texts = dobj->control.dtexts;
> +
> if (dobj->type != SND_SOC_DOBJ_ENUM ||
> dobj->control.kcontrol->put !=
> skl_tplg_multi_config_set_dmic)
>
> base-commit: 69fe63aa100220c8fd1f451dd54dd0895df1441d
Thanks for pointing out and fixing this. This check was obviously
missing there. I did a quick verification on few of our platforms,
encountered no issues, so:
Reviewed-by: Mateusz Gorski <[email protected]>
Also, could you please:
- describe the affected configuration (used machine driver or audio card
name, device name),
- share full dmesg logs from one of said crashes,
- copy the output of "amixer -c0 controls" command executed on affected
device.
These would be useful information for us to further improve our
validation and help us with debugging.
Thanks,
Mateusz
On 12/10/2020 1:14 PM, Lukasz Majczak wrote:
> + kcontrol = dobj->control.kcontrol;
> + if(!kcontrol)
> + continue;
Small nitpick, there should be space between if and opening parenthesis
as recommended by coding style.
Thanks,
Amadeusz
There is no check for the kcontrol against NULL and in some cases
it causes kernel to crash.
Fixes: 2d744ecf2b984 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHLT")
Cc: <[email protected]> # 5.4+
Signed-off-by: Lukasz Majczak <[email protected]>
Reviewed-by: Mateusz Gorski <[email protected]>
---
sound/soc/intel/skylake/skl-topology.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
v1 -> v2: fixed coding style
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index ae466cd592922..8f0bfda7096a9 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -3618,12 +3618,18 @@ static void skl_tplg_complete(struct snd_soc_component *component)
int i;
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 snd_kcontrol *kcontrol;
+ struct soc_enum *se;
+ char **texts;
char chan_text[4];
+ kcontrol = dobj->control.kcontrol;
+ if (!kcontrol)
+ continue;
+
+ se = (struct soc_enum *)kcontrol->private_value;
+ texts = dobj->control.dtexts;
+
if (dobj->type != SND_SOC_DOBJ_ENUM ||
dobj->control.kcontrol->put !=
skl_tplg_multi_config_set_dmic)
--
2.25.1
Hi,
This is just a kind reminder. Is there anything more required to
upstream this patch?
Best regards,
Lukasz
czw., 17 gru 2020 o 14:06 Lukasz Majczak <[email protected]> napisał(a):
>
> There is no check for the kcontrol against NULL and in some cases
> it causes kernel to crash.
>
> Fixes: 2d744ecf2b984 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHLT")
> Cc: <[email protected]> # 5.4+
> Signed-off-by: Lukasz Majczak <[email protected]>
> Reviewed-by: Mateusz Gorski <[email protected]>
> ---
> sound/soc/intel/skylake/skl-topology.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
> v1 -> v2: fixed coding style
>
> diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
> index ae466cd592922..8f0bfda7096a9 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -3618,12 +3618,18 @@ static void skl_tplg_complete(struct snd_soc_component *component)
> int i;
>
> 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 snd_kcontrol *kcontrol;
> + struct soc_enum *se;
> + char **texts;
> char chan_text[4];
>
> + kcontrol = dobj->control.kcontrol;
> + if (!kcontrol)
> + continue;
> +
> + se = (struct soc_enum *)kcontrol->private_value;
> + texts = dobj->control.dtexts;
> +
> if (dobj->type != SND_SOC_DOBJ_ENUM ||
> dobj->control.kcontrol->put !=
> skl_tplg_multi_config_set_dmic)
> --
> 2.25.1
>
Hi Pierre,
Is there anything more to do to get the ACK for this patch?
Best regards,
Lukasz
wt., 12 sty 2021 o 12:34 Łukasz Majczak <[email protected]> napisał(a):
>
> Hi,
>
> This is just a kind reminder. Is there anything more required to
> upstream this patch?
>
> Best regards,
> Lukasz
>
>
> czw., 17 gru 2020 o 14:06 Lukasz Majczak <[email protected]> napisał(a):
> >
> > There is no check for the kcontrol against NULL and in some cases
> > it causes kernel to crash.
> >
> > Fixes: 2d744ecf2b984 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHLT")
> > Cc: <[email protected]> # 5.4+
> > Signed-off-by: Lukasz Majczak <[email protected]>
> > Reviewed-by: Mateusz Gorski <[email protected]>
> > ---
> > sound/soc/intel/skylake/skl-topology.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> > v1 -> v2: fixed coding style
> >
> > diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
> > index ae466cd592922..8f0bfda7096a9 100644
> > --- a/sound/soc/intel/skylake/skl-topology.c
> > +++ b/sound/soc/intel/skylake/skl-topology.c
> > @@ -3618,12 +3618,18 @@ static void skl_tplg_complete(struct snd_soc_component *component)
> > int i;
> >
> > 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 snd_kcontrol *kcontrol;
> > + struct soc_enum *se;
> > + char **texts;
> > char chan_text[4];
> >
> > + kcontrol = dobj->control.kcontrol;
> > + if (!kcontrol)
> > + continue;
> > +
> > + se = (struct soc_enum *)kcontrol->private_value;
> > + texts = dobj->control.dtexts;
> > +
> > if (dobj->type != SND_SOC_DOBJ_ENUM ||
> > dobj->control.kcontrol->put !=
> > skl_tplg_multi_config_set_dmic)
> > --
> > 2.25.1
> >
On 1/20/21 9:49 AM, Łukasz Majczak wrote:
> Hi Pierre,
>
> Is there anything more to do to get the ACK for this patch?
Adding Cezary and Amadeusz for feedback, I can't pretend having any sort
of knowledge on the Skylake driver internals and topology usage.
> Best regards,
> Lukasz
>
> wt., 12 sty 2021 o 12:34 Łukasz Majczak <[email protected]> napisał(a):
>>
>> Hi,
>>
>> This is just a kind reminder. Is there anything more required to
>> upstream this patch?
>>
>> Best regards,
>> Lukasz
>>
>>
>> czw., 17 gru 2020 o 14:06 Lukasz Majczak <[email protected]> napisał(a):
>>>
>>> There is no check for the kcontrol against NULL and in some cases
>>> it causes kernel to crash.
>>>
>>> Fixes: 2d744ecf2b984 ("ASoC: Intel: Skylake: Automatic DMIC format configuration according to information from NHLT")
>>> Cc: <[email protected]> # 5.4+
>>> Signed-off-by: Lukasz Majczak <[email protected]>
>>> Reviewed-by: Mateusz Gorski <[email protected]>
>>> ---
>>> sound/soc/intel/skylake/skl-topology.c | 14 ++++++++++----
>>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>> v1 -> v2: fixed coding style
>>>
>>> diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
>>> index ae466cd592922..8f0bfda7096a9 100644
>>> --- a/sound/soc/intel/skylake/skl-topology.c
>>> +++ b/sound/soc/intel/skylake/skl-topology.c
>>> @@ -3618,12 +3618,18 @@ static void skl_tplg_complete(struct snd_soc_component *component)
>>> int i;
>>>
>>> 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 snd_kcontrol *kcontrol;
>>> + struct soc_enum *se;
>>> + char **texts;
>>> char chan_text[4];
>>>
>>> + kcontrol = dobj->control.kcontrol;
>>> + if (!kcontrol)
>>> + continue;
>>> +
>>> + se = (struct soc_enum *)kcontrol->private_value;
>>> + texts = dobj->control.dtexts;
>>> +
>>> if (dobj->type != SND_SOC_DOBJ_ENUM ||
>>> dobj->control.kcontrol->put !=
>>> skl_tplg_multi_config_set_dmic)
>>> --
>>> 2.25.1
>>>
On 2021-01-20 5:33 PM, Pierre-Louis Bossart wrote:
> On 1/20/21 9:49 AM, Łukasz Majczak wrote:
>> Hi Pierre,
>>
>> Is there anything more to do to get the ACK for this patch?
>
> Adding Cezary and Amadeusz for feedback, I can't pretend having any sort
> of knowledge on the Skylake driver internals and topology usage.
>
Thanks for the CC, Pierre.
...
>>>>
>>>> diff --git a/sound/soc/intel/skylake/skl-topology.c
>>>> b/sound/soc/intel/skylake/skl-topology.c
>>>> index ae466cd592922..8f0bfda7096a9 100644
>>>> --- a/sound/soc/intel/skylake/skl-topology.c
>>>> +++ b/sound/soc/intel/skylake/skl-topology.c
>>>> @@ -3618,12 +3618,18 @@ static void skl_tplg_complete(struct
>>>> snd_soc_component *component)
>>>> int i;
>>>>
>>>> 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 snd_kcontrol *kcontrol;
>>>> + struct soc_enum *se;
>>>> + char **texts;
>>>> char chan_text[4];
>>>>
>>>> + kcontrol = dobj->control.kcontrol;
>>>> + if (!kcontrol)
>>>> + continue;
>>>> +
>>>> + se = (struct soc_enum *)kcontrol->private_value;
>>>> + texts = dobj->control.dtexts;
>>>> +
>>>> if (dobj->type != SND_SOC_DOBJ_ENUM ||
>>>> dobj->control.kcontrol->put !=
>>>> skl_tplg_multi_config_set_dmic)
Just checked the history behind this. And must say, I liked Ricardo's
version better. Except for the "= {};" bit which Mark already pointed
out - it should be a separate fix - it's simply more optional
e.g.: 'kcontrol' gets assigned yet 'if' above is not updated accordingly:
s/dobj->control.kcontrol->put/kcontrol->put
Czarek
On 2021-01-20 5:41 PM, Rojewski, Cezary wrote:
>
> Just checked the history behind this. And must say, I liked Ricardo's
> version better. Except for the "= {};" bit which Mark already pointed
> out - it should be a separate fix - it's simply more optional
Meant to say: optimal.