Re: [PATCH] dmaengine: tegra210-adma: fix of_irq_get() error check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/31/2017 07:56 PM, Thierry Reding wrote:

of_irq_get() may return 0 as well as negative error number on failure,
while the driver only checks for the negative values. The driver would then
call request_irq(0, ...) in tegra_adma_alloc_chan_resources() and never get
valid channel interrupt.

Check for 'tdc->irq <= 0' instead and return -ENXIO from the driver's probe
iff of_irq_get() returned 0.

Fixes: f46b195799b5 ("dmaengine: tegra-adma: Add support for Tegra210 ADMA")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
[...]

Index: slave-dma/drivers/dma/tegra210-adma.c
===================================================================
--- slave-dma.orig/drivers/dma/tegra210-adma.c
+++ slave-dma/drivers/dma/tegra210-adma.c
@@ -717,8 +717,8 @@ static int tegra_adma_probe(struct platf
           tdc->chan_addr = tdma->base_addr + ADMA_CH_REG_OFFSET(i);
             tdc->irq = of_irq_get(pdev->dev.of_node, i);
-        if (tdc->irq < 0) {
-            ret = tdc->irq;
+        if (tdc->irq <= 0) {
+            ret = tdc->irq ?: -ENXIO;
               goto irq_dispose;
           }


I think in this particular case it would be better to just call
platform_get_irq(), which already implements the equivalent.

    Not that platform_get_irq() was no better (if not worse indeed, as it

   s/Not/Note/.

could return 0 both on failure ad success), until I fixed it back in 2016:

   s/ad/and/.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af

    OK, I'll try looking into that...

    I've looked at the commit history and no, I don't agree to use
platform_get_irq(): my platform_get_irq() fix landed in mainline later than
this driver, so fixing this bug in stable kernels would ensue an extra
dependency...

Well, surely the platform_get_irq() function should also be fixed in
stable kernels, so that all callers can be fixed at the same time.

It has been fixed in 3.16.y, 4.4.y and 4.8.y so far. I haven't received other stable mails yet.

That said, this does fix a bug, and it fixes it in the least invasive
way, and the damage I'm complaining about is preexisting, so:

Acked-by: Thierry Reding <treding@xxxxxxxxxx>

   Thank you!

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux