[alsa-devel] [PATCH] ASoC: dapm: Fix NULL pointer dereference in snd_soc_dapm_new_dai
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Thu Mar 21 14:59:55 CET 2019
On 3/21/19 7:31 AM, Mark Brown wrote:
> On Thu, Mar 21, 2019 at 03:41:25PM +0530, Pankaj Bharadiya wrote:
>
>> outfree_kcontrol_news:
>> devm_kfree(card->dev, (void *)template.kcontrol_news);
>> - snd_soc_dapm_free_kcontrol(card, &private_value,
>> - rtd->dai_link->num_params, w_param_text);
>> + if (w_param_text)
>> + snd_soc_dapm_free_kcontrol(card, &private_value,
>> + rtd->dai_link->num_params, w_param_text);
> This is very non-obvious - it's not at all clear why we'd need the text
> to free controls. If there is an issue here it seems like it'd be
> better to make sure that snd_soc_dapm_free_kcontrol() can cope with that
> being NULL, that will be clearer and also avoid potential issues with
> other callers.
I believe the issue is real, but you need to look at the entire code to figure it out
/* allocate memory for control, only in case of multiple configs */
if (rtd->dai_link->num_params > 1) {
w_param_text = devm_kcalloc(card->dev,
rtd->dai_link->num_params,
sizeof(char *), GFP_KERNEL);
if (!w_param_text) {
ret = -ENOMEM;
goto param_fail;
}
template.num_kcontrols = 1;
template.kcontrol_news =
snd_soc_dapm_alloc_kcontrol(card,
link_name,
rtd->dai_link->params,
rtd->dai_link->num_params,
w_param_text, &private_value);
if (!template.kcontrol_news) {
ret = -ENOMEM;
goto param_fail;
}
} else {
w_param_text = NULL; <<<< this is set when there is a single config
}
dev_dbg(card->dev, "ASoC: adding %s widget\n", link_name);
w = snd_soc_dapm_new_control_unlocked(&card->dapm, &template);
if (IS_ERR(w)) {
ret = PTR_ERR(w);
goto outfree_kcontrol_news; <<< the control creation failed
}
w->priv = rtd;
return w;
outfree_kcontrol_news:
devm_kfree(card->dev, (void *)template.kcontrol_news);
<<< and in the function below we try to access w_param_text and private_value which haven't been allocated.
snd_soc_dapm_free_kcontrol(card, &private_value,
rtd->dai_link->num_params, w_param_text);
That said I agree with Mark that it's better to change snd_soc_dapm_free_kcontrol directly.
More information about the Alsa-devel
mailing list