Re: [PATCH] ASoC: meson: Keep link pointers valid on realloc failure

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbinding the device causes a use-after-free because `meson_card_remove` frees `card->dai_link` manually before the devres callbacks unregister the card.
--

--- Patch [1]: [PATCH] ASoC: meson: Keep link pointers valid on realloc failure ---
commit 677ad75f5fb5a578b984e744f26d24b961c89685
Author: Linmao Li <[email protected]>

ASoC: meson: Keep link pointers valid on realloc failure

This commit fixes a potential use-after-free and double free in the error
path of meson_card_reallocate_links(). It updates the card->dai_link
pointer immediately after the first successful krealloc(), ensuring the
cleanup path can safely free the memory if the second krealloc() fails.

> diff --git a/sound/soc/meson/meson-card-utils.c b/sound/soc/meson/meson-card-utils.c
> index cdb759b466ad4..38c1b2ae227ff 100644
> --- a/sound/soc/meson/meson-card-utils.c
> +++ b/sound/soc/meson/meson-card-utils.c
> @@ -52,19 +52,18 @@ int meson_card_reallocate_links(struct snd_soc_card *card,
>  	if (!links)
>  		goto err_links;
>  
> +	priv->card.dai_link = links;
> +
>  	ldata = krealloc(priv->link_data,
>  			 num_links * sizeof(*priv->link_data),
>  			 GFP_KERNEL | __GFP_ZERO);
>  	if (!ldata)
> -		goto err_ldata;
> +		goto err_links;
>  
> -	priv->card.dai_link = links;
>  	priv->link_data = ldata;
>  	priv->card.num_links = num_links;
>  	return 0;
>  
> -err_ldata:
> -	kfree(links);
>  err_links:
>  	dev_err(priv->card.dev, "failed to allocate links\n");
>  	return -ENOMEM;

[Severity: High]
This isn't a bug introduced by this patch, but does unbinding the device
cause a use-after-free because meson_card_remove() frees card->dai_link
manually before the devres callbacks unregister the card?

When the driver is unbound, the driver core calls meson_card_remove(), which
calls meson_card_clean_references() to free the links:

sound/soc/meson/meson-card-utils.c:meson_card_clean_references() {
    ...
    kfree(card->dai_link);
    ...
}

Only after .remove returns does the driver core run devres callbacks, which
includes unregistering the card via devm_snd_soc_register_card's cleanup
handler.

The ASoC unregister path iterates over all PCM runtimes and dereferences
rtd->dai_link, which points directly to elements of the already freed
card->dai_link array.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic