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