[PATCH] ASoC: topology: Perform component check upfront

[Amadeusz Sławiński]

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 at linux.intel.com> 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 at linux.intel.com>
>> ---

(...)

>> @@ -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.

Amadeusz