* [PATCH] dmaengine: coh901318: Fix a double lock bug in dma_tc_handle()
@ 2020-02-17 14:40 Dan Carpenter
2020-02-17 22:24 ` Geert Uytterhoeven
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2020-02-17 14:40 UTC (permalink / raw)
To: Linus Walleij
Cc: kernel-janitors, Vinod Koul, Jia-Ju Bai, dmaengine, Dan Williams,
linux-arm-kernel
The caller is already holding the lock so this will deadlock.
Fixes: 0b58828c923e ("DMAENGINE: COH 901 318 remove irq counting")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is the second double lock bug found using static analysis. The
previous one was commit 627469e4445b ("dmaengine: coh901318: Fix a
double-lock bug").
The fact that this has been broken for ten years suggests that no one
has the hardware.
drivers/dma/coh901318.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
index e51d836afcc7..1092d4ce723e 100644
--- a/drivers/dma/coh901318.c
+++ b/drivers/dma/coh901318.c
@@ -1947,8 +1947,6 @@ static void dma_tc_handle(struct coh901318_chan *cohc)
return;
}
- spin_lock(&cohc->lock);
-
/*
* When we reach this point, at least one queue item
* should have been moved over from cohc->queue to
@@ -1969,8 +1967,6 @@ static void dma_tc_handle(struct coh901318_chan *cohc)
if (coh901318_queue_start(cohc) == NULL)
cohc->busy = 0;
- spin_unlock(&cohc->lock);
-
/*
* This tasklet will remove items from cohc->active
* and thus terminates them.
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: coh901318: Fix a double lock bug in dma_tc_handle()
2020-02-17 14:40 [PATCH] dmaengine: coh901318: Fix a double lock bug in dma_tc_handle() Dan Carpenter
@ 2020-02-17 22:24 ` Geert Uytterhoeven
2020-02-19 9:17 ` Vinod Koul
0 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2020-02-17 22:24 UTC (permalink / raw)
To: Dan Carpenter
Cc: Linus Walleij, kernel-janitors, Vinod Koul, Jia-Ju Bai,
dmaengine, Dan Williams, Linux ARM
Hi Dan,
On Mon, Feb 17, 2020 at 3:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> The caller is already holding the lock so this will deadlock.
>
> Fixes: 0b58828c923e ("DMAENGINE: COH 901 318 remove irq counting")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> This is the second double lock bug found using static analysis. The
> previous one was commit 627469e4445b ("dmaengine: coh901318: Fix a
> double-lock bug").
>
> The fact that this has been broken for ten years suggests that no one
> has the hardware.
Or this only runs CONFIG_SMP=n kernels?
This seems to be used in arch/arm/boot/dts/ste-u300.dts only, and
CONFIG_ARCH_U300 is a ARCH_MULTI_V5 platform, which looks like
it doesn't support SMP?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: coh901318: Fix a double lock bug in dma_tc_handle()
2020-02-17 22:24 ` Geert Uytterhoeven
@ 2020-02-19 9:17 ` Vinod Koul
2020-02-19 9:20 ` Geert Uytterhoeven
2020-02-21 14:50 ` Linus Walleij
0 siblings, 2 replies; 7+ messages in thread
From: Vinod Koul @ 2020-02-19 9:17 UTC (permalink / raw)
To: Geert Uytterhoeven, Linus Walleij
Cc: kernel-janitors, Jia-Ju Bai, Dan Carpenter, dmaengine,
Dan Williams, Linux ARM
On 17-02-20, 23:24, Geert Uytterhoeven wrote:
> Hi Dan,
>
> On Mon, Feb 17, 2020 at 3:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > The caller is already holding the lock so this will deadlock.
> >
> > Fixes: 0b58828c923e ("DMAENGINE: COH 901 318 remove irq counting")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > This is the second double lock bug found using static analysis. The
> > previous one was commit 627469e4445b ("dmaengine: coh901318: Fix a
> > double-lock bug").
> >
> > The fact that this has been broken for ten years suggests that no one
> > has the hardware.
>
> Or this only runs CONFIG_SMP=n kernels?
> This seems to be used in arch/arm/boot/dts/ste-u300.dts only, and
> CONFIG_ARCH_U300 is a ARCH_MULTI_V5 platform, which looks like
> it doesn't support SMP?
Should we drop the driver then..?
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: coh901318: Fix a double lock bug in dma_tc_handle()
2020-02-19 9:17 ` Vinod Koul
@ 2020-02-19 9:20 ` Geert Uytterhoeven
2020-02-19 9:27 ` Vinod Koul
2020-02-21 14:50 ` Linus Walleij
1 sibling, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2020-02-19 9:20 UTC (permalink / raw)
To: Vinod Koul
Cc: Linus Walleij, kernel-janitors, Jia-Ju Bai, Linux ARM, dmaengine,
Dan Williams, Dan Carpenter
Hi Vinod,
On Wed, Feb 19, 2020 at 10:18 AM Vinod Koul <vkoul@kernel.org> wrote:
> On 17-02-20, 23:24, Geert Uytterhoeven wrote:
> > On Mon, Feb 17, 2020 at 3:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > The caller is already holding the lock so this will deadlock.
> > >
> > > Fixes: 0b58828c923e ("DMAENGINE: COH 901 318 remove irq counting")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > This is the second double lock bug found using static analysis. The
> > > previous one was commit 627469e4445b ("dmaengine: coh901318: Fix a
> > > double-lock bug").
> > >
> > > The fact that this has been broken for ten years suggests that no one
> > > has the hardware.
> >
> > Or this only runs CONFIG_SMP=n kernels?
> > This seems to be used in arch/arm/boot/dts/ste-u300.dts only, and
> > CONFIG_ARCH_U300 is a ARCH_MULTI_V5 platform, which looks like
> > it doesn't support SMP?
>
> Should we drop the driver then..?
Why? Because spinlocks are no-ops on SMP=n, and spinlock bugs thus don't
affect the single platform using the driver?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: coh901318: Fix a double lock bug in dma_tc_handle()
2020-02-19 9:20 ` Geert Uytterhoeven
@ 2020-02-19 9:27 ` Vinod Koul
0 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2020-02-19 9:27 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, kernel-janitors, Jia-Ju Bai, Linux ARM, dmaengine,
Dan Williams, Dan Carpenter
On 19-02-20, 10:20, Geert Uytterhoeven wrote:
> Hi Vinod,
>
> On Wed, Feb 19, 2020 at 10:18 AM Vinod Koul <vkoul@kernel.org> wrote:
> > On 17-02-20, 23:24, Geert Uytterhoeven wrote:
> > > On Mon, Feb 17, 2020 at 3:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > > The caller is already holding the lock so this will deadlock.
> > > >
> > > > Fixes: 0b58828c923e ("DMAENGINE: COH 901 318 remove irq counting")
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > > This is the second double lock bug found using static analysis. The
> > > > previous one was commit 627469e4445b ("dmaengine: coh901318: Fix a
> > > > double-lock bug").
> > > >
> > > > The fact that this has been broken for ten years suggests that no one
> > > > has the hardware.
> > >
> > > Or this only runs CONFIG_SMP=n kernels?
> > > This seems to be used in arch/arm/boot/dts/ste-u300.dts only, and
> > > CONFIG_ARCH_U300 is a ARCH_MULTI_V5 platform, which looks like
> > > it doesn't support SMP?
> >
> > Should we drop the driver then..?
>
> Why? Because spinlocks are no-ops on SMP=n, and spinlock bugs thus don't
> affect the single platform using the driver?
That doesn't answer the question if anyone has a hardware and we have
users :)
Sorry I should have written better about hardware and testing
rather than cryptic reply which may have suggested about SMP :)
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: coh901318: Fix a double lock bug in dma_tc_handle()
2020-02-19 9:17 ` Vinod Koul
2020-02-19 9:20 ` Geert Uytterhoeven
@ 2020-02-21 14:50 ` Linus Walleij
2020-02-24 16:30 ` Vinod Koul
1 sibling, 1 reply; 7+ messages in thread
From: Linus Walleij @ 2020-02-21 14:50 UTC (permalink / raw)
To: Vinod Koul
Cc: kernel-janitors, Jia-Ju Bai, Geert Uytterhoeven, Linux ARM,
dmaengine, Dan Williams, Dan Carpenter
On Wed, Feb 19, 2020 at 10:17 AM Vinod Koul <vkoul@kernel.org> wrote:
> On 17-02-20, 23:24, Geert Uytterhoeven wrote:
> > On Mon, Feb 17, 2020 at 3:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > The caller is already holding the lock so this will deadlock.
> > >
> > > Fixes: 0b58828c923e ("DMAENGINE: COH 901 318 remove irq counting")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > This is the second double lock bug found using static analysis. The
> > > previous one was commit 627469e4445b ("dmaengine: coh901318: Fix a
> > > double-lock bug").
> > >
> > > The fact that this has been broken for ten years suggests that no one
> > > has the hardware.
> >
> > Or this only runs CONFIG_SMP=n kernels?
> > This seems to be used in arch/arm/boot/dts/ste-u300.dts only, and
> > CONFIG_ARCH_U300 is a ARCH_MULTI_V5 platform, which looks like
> > it doesn't support SMP?
>
> Should we drop the driver then..?
I still have the hardware and it still works if that is the question :D
And yeah it only has one CPU, but still has a DMA engine.
The patch is fine to apply because it fixes a bug, should the same
hardware block be used on SMP.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: coh901318: Fix a double lock bug in dma_tc_handle()
2020-02-21 14:50 ` Linus Walleij
@ 2020-02-24 16:30 ` Vinod Koul
0 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2020-02-24 16:30 UTC (permalink / raw)
To: Linus Walleij
Cc: kernel-janitors, Jia-Ju Bai, Geert Uytterhoeven, Linux ARM,
dmaengine, Dan Williams, Dan Carpenter
On 21-02-20, 15:50, Linus Walleij wrote:
> On Wed, Feb 19, 2020 at 10:17 AM Vinod Koul <vkoul@kernel.org> wrote:
> > On 17-02-20, 23:24, Geert Uytterhoeven wrote:
> > > On Mon, Feb 17, 2020 at 3:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > > The caller is already holding the lock so this will deadlock.
> > > >
> > > > Fixes: 0b58828c923e ("DMAENGINE: COH 901 318 remove irq counting")
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > > This is the second double lock bug found using static analysis. The
> > > > previous one was commit 627469e4445b ("dmaengine: coh901318: Fix a
> > > > double-lock bug").
> > > >
> > > > The fact that this has been broken for ten years suggests that no one
> > > > has the hardware.
> > >
> > > Or this only runs CONFIG_SMP=n kernels?
> > > This seems to be used in arch/arm/boot/dts/ste-u300.dts only, and
> > > CONFIG_ARCH_U300 is a ARCH_MULTI_V5 platform, which looks like
> > > it doesn't support SMP?
> >
> > Should we drop the driver then..?
>
> I still have the hardware and it still works if that is the question :D
Thanks for confirming :)
>
> And yeah it only has one CPU, but still has a DMA engine.
>
> The patch is fine to apply because it fixes a bug, should the same
> hardware block be used on SMP.
Applied now.
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-24 16:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-17 14:40 [PATCH] dmaengine: coh901318: Fix a double lock bug in dma_tc_handle() Dan Carpenter
2020-02-17 22:24 ` Geert Uytterhoeven
2020-02-19 9:17 ` Vinod Koul
2020-02-19 9:20 ` Geert Uytterhoeven
2020-02-19 9:27 ` Vinod Koul
2020-02-21 14:50 ` Linus Walleij
2020-02-24 16:30 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).