On Thu, Mar 12, 2020 at 7:11 AM Amadeusz Sławiński < amadeuszx.slawinski@xxxxxxxxxxxxxxx> wrote: > > > On 3/12/2020 2:57 PM, Sridharan, Ranjani wrote: > > 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> > >> --- > > (...) > > >> @@ -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 > > > Well it can happen if you pass it wrong pointer for some reason (don't > ask :P), I think it's better to have check than none at all. > If you pass it NULL pointer to component it can parse part of a file and > then suddenly give you NULL pointer dereference in some seemingly > "random" function. I would say it's easier for programmer to understand > what happens and use such functionality if it performs such check upfront. Yes, I suppose it is not a bad idea to have the check for robustness. So, Reviewed-by: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx>
