linux-kernel - Re: [PATCH] ALSA: lx6464es: add error handling for pci_ioremap

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:   Tue, 12 Jun 2018 08:45:56 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     "Zhouyang Jia" <jiazhouyang09@...il.com>
Cc:     <alsa-devel@...a-project.org>,
        "Bhumika Goyal" <bhumirks@...il.com>,
        "Jaroslav Kysela" <perex@...ex.cz>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ALSA: lx6464es: add error handling for pci_ioremap_bar

On Tue, 12 Jun 2018 05:23:14 +0200,
Zhouyang Jia wrote:
> 
> When pci_ioremap_bar fails, the lack of error-handling code may
> cause unexpected results.
> 
> This patch adds error-handling code after calling pci_ioremap_bar.
> 
> Signed-off-by: Zhouyang Jia <jiazhouyang09@...il.com>

This patch makes no sense, sorry.  The whole lx6464es error handling
there is buggy.  We need to fix it more properly.

And, you must have noticed a bad smell in that change before
submitting this.  Basically, using the same goto label for the error
handling of the next step is already fishy.  In this case, you'd have
to unmap at the error path after this point.  If you can use the same
label, it means that the unmap is missing -- already something wrong.


thanks,

Takashi

> ---
>  sound/pci/lx6464es/lx6464es.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c
> index 9655b08..6157b6d 100644
> --- a/sound/pci/lx6464es/lx6464es.c
> +++ b/sound/pci/lx6464es/lx6464es.c
> @@ -1016,6 +1016,10 @@ static int snd_lx6464es_create(struct snd_card *card,
>  
>  	/* dsp port */
>  	chip->port_dsp_bar = pci_ioremap_bar(pci, 2);
> +	if (!chip->port_dsp_bar) {
> +		dev_err(card->dev, "cannot remap PCI memory region\n");
> +		goto request_irq_failed;
> +	}
>  
>  	err = request_threaded_irq(pci->irq, lx_interrupt, lx_threaded_irq,
>  				   IRQF_SHARED, KBUILD_MODNAME, chip);
> -- 
> 2.7.4
> 
>