linux-kernel - Re: Public ridicule due to sound/soc/soc-core.c abuse of the driver model

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 9 Jan 2012 21:37:24 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Cc:	Greg KH <greg@...ah.com>,
	Frank Mandarino <fmandarino@...relia.com>,
	Liam Girdwood <lrg@...com>, Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.de>, alsa-devel@...a-project.org,
	linux-kernel@...r.kernel.org
Subject: Re: Public ridicule due to sound/soc/soc-core.c abuse of the
	driver model

On Mon, Jan 09, 2012 at 12:31:30PM -0800, Mark Brown wrote:
> If it's code like the rtd devices which is reasonably sensible and
> stable that's one thing but wading into code we know is fragile for
> what's essentially a warning fix is completely disproportionate.

You're still under the impression that it's "just a warning" and not
"a potential oops".

And I disagree with your assessment of how easy it is to fix each
problem.

The rtd devices are stored in an array - this array needs breaking
up into separate allocations for each rtd as the very first step in
fixing that.  That then needs more error checking added, etc.

On the other hand, the AC'97 code in ASoC should need this to fix it:

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index a25fa63..e85ae4d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -452,14 +452,10 @@ static inline void soc_cleanup_card_debugfs(struct snd_soc_card *card)
 /* unregister ac97 codec */
 static int soc_ac97_dev_unregister(struct snd_soc_codec *codec)
 {
-	if (codec->ac97->dev.bus)
-		device_unregister(&codec->ac97->dev);
+	device_del(&codec->ac97->dev);
 	return 0;
 }
 
-/* stop no dev release warning */
-static void soc_ac97_device_release(struct device *dev){}
-
 /* register ac97 codec to bus */
 static int soc_ac97_dev_register(struct snd_soc_codec *codec)
 {
@@ -467,14 +463,12 @@ static int soc_ac97_dev_register(struct snd_soc_codec *codec)
 
 	codec->ac97->dev.bus = &ac97_bus_type;
 	codec->ac97->dev.parent = codec->card->dev;
-	codec->ac97->dev.release = soc_ac97_device_release;
 
 	dev_set_name(&codec->ac97->dev, "%d-%d:%s",
 		     codec->card->snd_card->number, 0, codec->name);
-	err = device_register(&codec->ac97->dev);
+	err = device_add(&codec->ac97->dev);
 	if (err < 0) {
 		snd_printk(KERN_ERR "Can't register ac97 bus\n");
-		codec->ac97->dev.bus = NULL;
 		return err;
 	}
 	return 0;
@@ -1729,6 +1723,12 @@ int snd_soc_platform_write(struct snd_soc_platform *platform,
 }
 EXPORT_SYMBOL_GPL(snd_soc_platform_write);
 
+static void soc_ac97_device_release(struct device *dev)
+{
+	struct snd_ac97 *ac97 = container_of(dev, struct snd_ac97, dev);
+	kfree(ac97);
+}
+
 /**
  * snd_soc_new_ac97_codec - initailise AC97 device
  * @codec: audio codec
@@ -1756,6 +1756,9 @@ int snd_soc_new_ac97_codec(struct snd_soc_codec *codec,
 		return -ENOMEM;
 	}
 
+	codec->ac97->dev.release = soc_ac97_device_release;
+	device_initialize(&codec->ac97->dev);
+
 	codec->ac97->bus->ops = ops;
 	codec->ac97->num = num;
 
@@ -1783,7 +1786,7 @@ void snd_soc_free_ac97_codec(struct snd_soc_codec *codec)
 	soc_unregister_ac97_dai_link(codec);
 #endif
 	kfree(codec->ac97->bus);
-	kfree(codec->ac97);
+	put_device(&codec->ac97->dev);
 	codec->ac97 = NULL;
 	codec->ac97_created = 0;
 	mutex_unlock(&codec->mutex);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/