On Thu, Mar 12, 2020 at 7:11 AM Amadeusz Sławiński < amadeuszx.slawinski@linux.intel.com> 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@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@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 whileparsing */
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@linux.intel.com