[alsa-devel] [PATCH] ALSA: pci: Fix memory leak in snd_korg1212_create
In the implementation of snd_korg1212_create() the allocated memory for korg1212 is leaked in cases of error. Release korg1212 via snd_korg1212_free() if either of these calls fail: snd_korg1212_downloadDSPCode(), snd_pcm_new(), or snd_ctl_add().
Signed-off-by: Navid Emamdoost navid.emamdoost@gmail.com --- sound/pci/korg1212/korg1212.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/sound/pci/korg1212/korg1212.c b/sound/pci/korg1212/korg1212.c index 0d81eac0a478..e976e857d915 100644 --- a/sound/pci/korg1212/korg1212.c +++ b/sound/pci/korg1212/korg1212.c @@ -2367,8 +2367,10 @@ static int snd_korg1212_create(struct snd_card *card, struct pci_dev *pci,
mdelay(CARD_BOOT_DELAY_IN_MS);
- if (snd_korg1212_downloadDSPCode(korg1212)) + if (snd_korg1212_downloadDSPCode(korg1212)) { + snd_korg1212_free(korg1212); return -EBUSY; + }
K1212_DEBUG_PRINTK("korg1212: dspMemPhy = %08x U[%08x], " "PlayDataPhy = %08x L[%08x]\n" @@ -2383,8 +2385,11 @@ static int snd_korg1212_create(struct snd_card *card, struct pci_dev *pci, korg1212->RoutingTablePhy, LowerWordSwap(korg1212->RoutingTablePhy), korg1212->AdatTimeCodePhy, LowerWordSwap(korg1212->AdatTimeCodePhy));
- if ((err = snd_pcm_new(korg1212->card, "korg1212", 0, 1, 1, &korg1212->pcm)) < 0) + err = snd_pcm_new(korg1212->card, "korg1212", 0, 1, 1, &korg1212->pcm); + if (err < 0) { + snd_korg1212_free(korg1212); return err; + }
korg1212->pcm->private_data = korg1212; korg1212->pcm->private_free = snd_korg1212_free_pcm; @@ -2398,8 +2403,10 @@ static int snd_korg1212_create(struct snd_card *card, struct pci_dev *pci,
for (i = 0; i < ARRAY_SIZE(snd_korg1212_controls); i++) { err = snd_ctl_add(korg1212->card, snd_ctl_new1(&snd_korg1212_controls[i], korg1212)); - if (err < 0) + if (err < 0) { + snd_korg1212_free(korg1212); return err; + } }
snd_korg1212_proc_init(korg1212);
…
+++ b/sound/pci/korg1212/korg1212.c
…
@@ -2398,8 +2403,10 @@ static int snd_korg1212_create(struct snd_card *card, struct pci_dev *pci,
for (i = 0; i < ARRAY_SIZE(snd_korg1212_controls); i++) { err = snd_ctl_add(korg1212->card, snd_ctl_new1(&snd_korg1212_controls[i], korg1212));
if (err < 0)
if (err < 0) {snd_korg1212_free(korg1212); return err;}
I suggest to add a jump target according to the Linux coding style so that duplicate exception handling code can be reduced.
return 0;
+free_korg: + snd_korg1212_free(korg1212); + return err; }
Regards, Markus
I suggest to add a jump target according to the Linux coding style so that duplicate exception handling code can be reduced.
Do you find any information interesting from the update suggestion “ALSA: korg1212: Use common error handling code in two functions”? https://lore.kernel.org/alsa-devel/2165666c-69f9-c716-8ee8-f5071a41f37d@user... https://lore.kernel.org/patchwork/patch/851723/ https://lkml.org/lkml/2017/11/16/193
Regards, Markus
On Sun, 27 Oct 2019 20:12:04 +0100, Navid Emamdoost wrote:
In the implementation of snd_korg1212_create() the allocated memory for korg1212 is leaked in cases of error. Release korg1212 via snd_korg1212_free() if either of these calls fail: snd_korg1212_downloadDSPCode(), snd_pcm_new(), or snd_ctl_add().
This also leads to the double-free. The code path is after snd_device_new() which has its own destructor callback.
thanks,
Takashi
Signed-off-by: Navid Emamdoost navid.emamdoost@gmail.com
sound/pci/korg1212/korg1212.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/sound/pci/korg1212/korg1212.c b/sound/pci/korg1212/korg1212.c index 0d81eac0a478..e976e857d915 100644 --- a/sound/pci/korg1212/korg1212.c +++ b/sound/pci/korg1212/korg1212.c @@ -2367,8 +2367,10 @@ static int snd_korg1212_create(struct snd_card *card, struct pci_dev *pci,
mdelay(CARD_BOOT_DELAY_IN_MS);
if (snd_korg1212_downloadDSPCode(korg1212))
if (snd_korg1212_downloadDSPCode(korg1212)) {
snd_korg1212_free(korg1212); return -EBUSY;}
K1212_DEBUG_PRINTK("korg1212: dspMemPhy = %08x U[%08x], " "PlayDataPhy = %08x L[%08x]\n"@@ -2383,8 +2385,11 @@ static int snd_korg1212_create(struct snd_card *card, struct pci_dev *pci, korg1212->RoutingTablePhy, LowerWordSwap(korg1212->RoutingTablePhy), korg1212->AdatTimeCodePhy, LowerWordSwap(korg1212->AdatTimeCodePhy));
if ((err = snd_pcm_new(korg1212->card, "korg1212", 0, 1, 1, &korg1212->pcm)) < 0)
err = snd_pcm_new(korg1212->card, "korg1212", 0, 1, 1, &korg1212->pcm);
if (err < 0) {
snd_korg1212_free(korg1212); return err;}
korg1212->pcm->private_data = korg1212; korg1212->pcm->private_free = snd_korg1212_free_pcm;
@@ -2398,8 +2403,10 @@ static int snd_korg1212_create(struct snd_card *card, struct pci_dev *pci,
for (i = 0; i < ARRAY_SIZE(snd_korg1212_controls); i++) { err = snd_ctl_add(korg1212->card, snd_ctl_new1(&snd_korg1212_controls[i], korg1212));
if (err < 0)
if (err < 0) {snd_korg1212_free(korg1212); return err;} } snd_korg1212_proc_init(korg1212);-- 2.17.1
The code path is after snd_device_new() which has its own destructor callback.
Thanks for your reminder.
Can the properties of this programming interface be documented also together with the function declaration for the safer handling of ALSA components? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Can any more API information help to improve automatic source code analysis around similar functions?
Regards, Markus
On Mon, 28 Oct 2019 10:00:21 +0100, Markus Elfring wrote:
The code path is after snd_device_new() which has its own destructor callback.
Thanks for your reminder.
Can the properties of this programming interface be documented also together with the function declaration for the safer handling of ALSA components? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
I can think of only adding some comment at each point mentioning that the resource will be managed by the device destructor.
Can any more API information help to improve automatic source code analysis around similar functions?
If anything we can add, let me know.
Takashi
Can the properties of this programming interface be documented also together with the function declaration for the safer handling of ALSA components? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
I can think of only adding some comment at each point mentioning that the resource will be managed by the device destructor.
* Which places will be corresponding update candidates?
* Would you like to choose the wording?
Can any more API information help to improve automatic source code analysis around similar functions?
If anything we can add, let me know.
Will any tags become more helpful also for the software documentation?
Regards, Markus
On Mon, 28 Oct 2019 15:40:09 +0100, Markus Elfring wrote:
Can the properties of this programming interface be documented also together with the function declaration for the safer handling of ALSA components? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
I can think of only adding some comment at each point mentioning that the resource will be managed by the device destructor.
- Which places will be corresponding update candidates?
At each place where this kind of code is seen.
- Would you like to choose the wording?
I myself don't mind as long as it's clearly understandable for human being.
Can any more API information help to improve automatic source code analysis around similar functions?
If anything we can add, let me know.
Will any tags become more helpful also for the software documentation?
It's my question.
thanks,
Takashi
participants (3)
-
Markus Elfring -
Navid Emamdoost -
Takashi Iwai