| Message ID | 20201217130439.141943-1-lma@semihalf.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ASoC: Intel: Skylake: Check the kcontrol against NULL | expand |
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 <lma@semihalf.com> 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: <stable@vger.kernel.org> # 5.4+ > Signed-off-by: Lukasz Majczak <lma@semihalf.com> > Reviewed-by: Mateusz Gorski <mateusz.gorski@linux.intel.com> > --- > 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 <lma@semihalf.com> 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 <lma@semihalf.com> 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: <stable@vger.kernel.org> # 5.4+ > > Signed-off-by: Lukasz Majczak <lma@semihalf.com> > > Reviewed-by: Mateusz Gorski <mateusz.gorski@linux.intel.com> > > --- > > 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 <lma@semihalf.com> 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 <lma@semihalf.com> 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: <stable@vger.kernel.org> # 5.4+ >>> Signed-off-by: Lukasz Majczak <lma@semihalf.com> >>> Reviewed-by: Mateusz Gorski <mateusz.gorski@linux.intel.com> >>> --- >>> 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.
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)
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> Reviewed-by: Mateusz Gorski <mateusz.gorski@linux.intel.com> --- sound/soc/intel/skylake/skl-topology.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) v1 -> v2: fixed coding style