On Thu, Mar 12, 2020 at 3:14 AM Amadeusz Sławiński <
amadeuszx.slawinski@xxxxxxxxxxxxxxx> wrote:
> Function soc_tplg_dbytes_create(), calls soc_tplg_init_kcontrol() to
> perform additional driver specific initialization. While
> soc_tplg_init_kcontrol() ensures that component is valid before invoking
> ops->control_load, there is no such check at the end of
> soc_tplg_dbytes_create() where list_add() is used.
>
> Also in quite a few places, there is reference of tplg->comp->dapm or
> tplg->comp->card, without any checks for tplg->comp.
>
> In consequence of the above this may lead to referencing NULL pointer.
>
> This allows for removal of now unnecessary checks.
>
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
> ---
> sound/soc/soc-topology.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
> index 575da6aba807..66958c57d884 100644
> --- a/sound/soc/soc-topology.c
> +++ b/sound/soc/soc-topology.c
> @@ -251,7 +251,7 @@ static int soc_tplg_vendor_load_(struct soc_tplg *tplg,
> {
> int ret = 0;
>
> - if (tplg->comp && tplg->ops && tplg->ops->vendor_load)
> + if (tplg->ops && tplg->ops->vendor_load)
> ret = tplg->ops->vendor_load(tplg->comp, tplg->index, hdr);
> else {
> dev_err(tplg->dev, "ASoC: no vendor load callback for ID
> %d\n",
> @@ -283,7 +283,7 @@ static int soc_tplg_vendor_load(struct soc_tplg *tplg,
> static int soc_tplg_widget_load(struct soc_tplg *tplg,
> struct snd_soc_dapm_widget *w, struct snd_soc_tplg_dapm_widget
> *tplg_w)
> {
> - if (tplg->comp && tplg->ops && tplg->ops->widget_load)
> + if (tplg->ops && tplg->ops->widget_load)
> return tplg->ops->widget_load(tplg->comp, tplg->index, w,
> tplg_w);
>
> @@ -295,7 +295,7 @@ static int soc_tplg_widget_load(struct soc_tplg *tplg,
> static int soc_tplg_widget_ready(struct soc_tplg *tplg,
> struct snd_soc_dapm_widget *w, struct snd_soc_tplg_dapm_widget
> *tplg_w)
> {
> - if (tplg->comp && tplg->ops && tplg->ops->widget_ready)
> + if (tplg->ops && tplg->ops->widget_ready)
> return tplg->ops->widget_ready(tplg->comp, tplg->index, w,
> tplg_w);
>
> @@ -307,7 +307,7 @@ static int soc_tplg_dai_load(struct soc_tplg *tplg,
> struct snd_soc_dai_driver *dai_drv,
> struct snd_soc_tplg_pcm *pcm, struct snd_soc_dai *dai)
> {
> - if (tplg->comp && tplg->ops && tplg->ops->dai_load)
> + if (tplg->ops && tplg->ops->dai_load)
> return tplg->ops->dai_load(tplg->comp, tplg->index,
> dai_drv,
> pcm, dai);
>
> @@ -318,7 +318,7 @@ static int soc_tplg_dai_load(struct soc_tplg *tplg,
> static int soc_tplg_dai_link_load(struct soc_tplg *tplg,
> struct snd_soc_dai_link *link, struct snd_soc_tplg_link_config
> *cfg)
> {
> - if (tplg->comp && tplg->ops && tplg->ops->link_load)
> + if (tplg->ops && tplg->ops->link_load)
> return tplg->ops->link_load(tplg->comp, tplg->index, link,
> cfg);
>
> return 0;
> @@ -327,7 +327,7 @@ static int soc_tplg_dai_link_load(struct soc_tplg
> *tplg,
> /* tell the component driver that all firmware has been loaded in this
> request */
> static void soc_tplg_complete(struct soc_tplg *tplg)
> {
> - if (tplg->comp && tplg->ops && tplg->ops->complete)
> + if (tplg->ops && tplg->ops->complete)
> tplg->ops->complete(tplg->comp);
> }
>
> @@ -684,7 +684,7 @@ EXPORT_SYMBOL_GPL(snd_soc_tplg_widget_bind_event);
> static int soc_tplg_init_kcontrol(struct soc_tplg *tplg,
> struct snd_kcontrol_new *k, struct snd_soc_tplg_ctl_hdr *hdr)
> {
> - if (tplg->comp && tplg->ops && tplg->ops->control_load)
> + if (tplg->ops && tplg->ops->control_load)
> return tplg->ops->control_load(tplg->comp, tplg->index, k,
> hdr);
>
> @@ -1174,7 +1174,7 @@ static int soc_tplg_kcontrol_elems_load(struct
> soc_tplg *tplg,
> static int soc_tplg_add_route(struct soc_tplg *tplg,
> struct snd_soc_dapm_route *route)
> {
> - if (tplg->comp && tplg->ops && tplg->ops->dapm_route_load)
> + if (tplg->ops && tplg->ops->dapm_route_load)
> return tplg->ops->dapm_route_load(tplg->comp, tplg->index,
> route);
>
> @@ -2564,7 +2564,7 @@ static int soc_tplg_manifest_load(struct soc_tplg
> *tplg,
> }
>
> /* pass control to component driver for optional further init */
> - if (tplg->comp && tplg->ops && tplg->ops->manifest)
> + if (tplg->ops && tplg->ops->manifest)
> ret = tplg->ops->manifest(tplg->comp, tplg->index,
> _manifest);
>
> if (!abi_match) /* free the duplicated one */
> @@ -2736,6 +2736,10 @@ int snd_soc_tplg_component_load(struct
> snd_soc_component *comp,
> struct soc_tplg tplg;
> int ret;
>
> + /* component needs to exist to keep and reference data while
> parsing */
> + if (!comp)
> + return -EINVAL;
> +
Amadeusz,
Thanks for this change. I agree that the checks for tplg->comp in the above
functions should be removed but is the check here really necessary at all?
snd_soc_tplg_component_load() is typically called when a component is
probed. Can it ever be null at all if it is getting probed?
Thanks,
Ranjani
