| Message ID | 20201210121438.7718-1-lma@semihalf.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: Intel: Skylake: Check the kcontrol against NULL | expand |
> 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: <stable@vger.kernel.org> # 5.4+ > Signed-off-by: Lukasz Majczak <lma@semihalf.com> > --- > 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 <mateusz.gorski@linux.intel.com> 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 Thu, Dec 10, 2020 at 7:55 AM Gorski, Mateusz < mateusz.gorski@linux.intel.com> wrote: > > > 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: <stable@vger.kernel.org> # 5.4+ > > Signed-off-by: Lukasz Majczak <lma@semihalf.com> > > --- > > 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 <mateusz.gorski@linux.intel.com> > > > Also, could you please: > > - describe the affected configuration (used machine driver or audio card > name, device name), > Primarily Google Pixelbook. However, this can happen whenever dobj->type != SND_SOC_DOBJ_ENUM. For many of those other types, kcontrol is not set. It is pure luck that the problem is not seen everywhere, and it seems to be compiler dependent. [Some compilers or compiler versions only assign se when needed, ie after the if() statement]. Guenter > - 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
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)
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: <stable@vger.kernel.org> # 5.4+ Signed-off-by: Lukasz Majczak <lma@semihalf.com> --- sound/soc/intel/skylake/skl-topology.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) base-commit: 69fe63aa100220c8fd1f451dd54dd0895df1441d