linux-kernel - Re: [PATCH] 8250_lpss: check null return when calling 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:   Fri, 19 Jul 2019 16:37:35 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Navid Emamdoost <navid.emamdoost@...il.com>,
        Kangjie Lu <kjlu@....edu>, Aditya Pakki <pakki001@....edu>
Cc:     emamd001@....edu, smccaman@....edu,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, Vinod Koul <vkoul@...nel.org>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] 8250_lpss: check null return when calling pci_ioremap_bar

On Thu, Jul 18, 2019 at 09:54:42PM -0500, Navid Emamdoost wrote:
> pci_ioremap_bar may return null. This is eventually de-referenced at 
> drivers/dma/dw/core.c:1154 and drivers/dma/dw/core.c:1168. A null check is
>  needed to prevent null de-reference. I am adding the check and in case of
>  failure returning -ENOMEM (I am not sure this is the best errno, you may 
> consider it as a placeholder), and subsequently changing the caller’s 
> return type, and propagating the error.

Thanks for the patch, my comments below.

>  	chip->irq = pci_irq_vector(pdev, 0);
>  	chip->regs = pci_ioremap_bar(pdev, 1);
> +	if (!chip->regs)
> +		return -ENOMEM;

This is the same case as below, it's fine to go on without DMA support.

>  	chip->pdata = &qrk_serial_dma_pdata;

So, I would rather to put like this...

Hold on, I remember someone already tried to fix this [1].

I dunno why it wasn't v5, due to [2].

Also, similar to yours, but wrong [3].

Thus, please, collaborate guys, and send one compiling solution  based on [1].

>  	/* Falling back to PIO mode if DMA probing fails */
>  	ret = dw_dma_probe(chip);
>  	if (ret)
> -		return;
> +		return 0;

[1]: https://www.spinics.net/lists/linux-serial/msg33965.html
[2]: https://lists.01.org/pipermail/kbuild-all/2019-March/059215.html
[3]: https://lore.kernel.org/patchwork/patch/1051000/

-- 
With Best Regards,
Andy Shevchenko