Re: [alsa-devel] [PATCH] ASoC: soc-core: Fix null pointer dereference in soc_find_component

[Curtis Malainey <cujomalainey@google.com>]

[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]

I have a patch to fix the memory leak but I haven't been able to test
it yet because I am remote right now and I accidentally bootlooped the
AMD device I am working on. I will have this tested early next week.
Here is the patch for anyone interested.

Curtis Malainey | Software Engineer | cujomalainey@google.com | 650-898-3849


On Fri, Jan 25, 2019 at 3:26 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Jan 24, 2019 at 01:07:17PM -0600, Pierre-Louis Bossart wrote:
>
> > Thanks for the feedback, will send a formal patch with the helper and
> > machine driver changes after I test more with the legacy drivers. Do you
> > have a preference for one patch that deals with multiple machines drivers in
> > one shot, or individual patches? The latter are nicer for backports (e.g.
> > for Chrome), the former nicer for maintainers...
>
> More patches is good, it doesn't make a huge difference if I get one big
> patch or a series of repetitive patches - big serieses are more of an
> issue if they're all different patches needing individual review.

[-- Attachment #2: 0001-ASoC-soc-core-clear-platform-pointers-on-error.patch --]
[-- Type: application/octet-stream, Size: 2404 bytes --]

From f5a22ae376188ee1ad54efd1274940295245eef3 Mon Sep 17 00:00:00 2001
From: Curtis Malainey <cujomalainey@chromium.org>
Date: Tue, 22 Jan 2019 21:23:21 -0800
Subject: [PATCH] ASoC: soc-core: clear platform pointers on error

Originally snd_soc_init_platform was not cleaning up its pointers, this
was fixed to always reallocate dynamic memory but created a memory leak
when snd_soc_init_platform was called multiple times during the same
probe attempt and also threw away any changes made to the struct between
calls. In order to acoid reallocating memory that is still valid, the
behaviour will be changed to clear the dynamically set pointers on a
probe error and a unregister event and snd_soc_init_platform will go
back to its original behaviour of only allocating null pointers so it will
stop throwing away valid changes.

Change-Id: Ibdeac041868dec095588cd339e2499569e35ef5c
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/soc-core.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 8a58fa86675a..b7354ea4725e 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1049,7 +1049,7 @@ static int snd_soc_init_platform(struct snd_soc_card *card,
 	 *	soc.h :: struct snd_soc_dai_link
 	 */
 	/* convert Legacy platform link */
-	if (!platform || dai_link->legacy_platform) {
+	if (!platform) {
 		platform = devm_kzalloc(card->dev,
 				sizeof(struct snd_soc_dai_link_component),
 				GFP_KERNEL);
@@ -1072,6 +1072,24 @@ static int snd_soc_init_platform(struct snd_soc_card *card,
 	return 0;
 }
 
+static void soc_cleanup_platform(struct snd_soc_card *card)
+{
+	struct snd_soc_dai_link *link;
+	int i;
+	/*
+	 * FIXME
+	 *
+	 * this function should be removed with snd_soc_init_platform
+	 */
+
+	for_each_card_prelinks(card, i, link) {
+		if (link->legacy_platform) {
+			link->legacy_platform = 0;
+			link->platforms       = NULL;
+		}
+	}
+}
+
 static int snd_soc_init_multicodec(struct snd_soc_card *card,
 				   struct snd_soc_dai_link *dai_link)
 {
@@ -2016,6 +2034,7 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card)
 	/* remove and free each DAI */
 	soc_remove_dai_links(card);
 	soc_remove_pcm_runtimes(card);
+	soc_cleanup_platform(card);
 
 	/* remove auxiliary devices */
 	soc_remove_aux_devices(card);
-- 
2.20.1.321.g9e740568ce-goog