Re: [PATCH] ASoC: dapm: Fix NULL pointer dereference in snd_soc_dapm_new_dai

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux