| Message ID | Pine.LNX.4.64.1401201320490.12653@axis700.grange (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Vinod Koul |
| Headers | show |
On Mon, Jan 20, 2014 at 1:27 PM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > Do not dereference descriptor pointer outside of spinlock. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > --- > > This is untested, respectively, never observed in life, but it seems to > me, that while the driver avoids reading callback and callback_param > outside of spinlock, same caution should be taken, when reading flags, but > maybe I'm wrong, feel free to drop then. But it's a nice optimization anyway! Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 20, 2014 at 01:27:45PM +0100, Guennadi Liakhovetski wrote: > Do not dereference descriptor pointer outside of spinlock. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > --- > > This is untested, respectively, never observed in life, but it seems to > me, that while the driver avoids reading callback and callback_param > outside of spinlock, same caution should be taken, when reading flags, but > maybe I'm wrong, feel free to drop then. > > drivers/dma/ste_dma40.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c > index b8c031b..6f4cb73 100644 > --- a/drivers/dma/ste_dma40.c > +++ b/drivers/dma/ste_dma40.c > @@ -1668,7 +1668,8 @@ static void dma_tasklet(unsigned long data) > } > > /* Callback to client */ > - callback = d40d->txd.callback; > + callback = (d40d->txd.flags & DMA_PREP_INTERRUPT) ? d40d->txd.callback : > + NULL; this assumes that someone is able to modify the descriptor flags. The dmac driver shouldnt do that. The client shouldn't do anything with descriptor after it has submitted it. So in that case, do we still have a condition where we may have an issue here :)
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index b8c031b..6f4cb73 100644 --- a/drivers/dma/ste_dma40.c +++ b/drivers/dma/ste_dma40.c @@ -1668,7 +1668,8 @@ static void dma_tasklet(unsigned long data) } /* Callback to client */ - callback = d40d->txd.callback; + callback = (d40d->txd.flags & DMA_PREP_INTERRUPT) ? d40d->txd.callback : + NULL; callback_param = d40d->txd.callback_param; if (!d40d->cyclic) { @@ -1690,7 +1691,7 @@ static void dma_tasklet(unsigned long data) spin_unlock_irqrestore(&d40c->lock, flags); - if (callback && (d40d->txd.flags & DMA_PREP_INTERRUPT)) + if (callback) callback(callback_param); return;
Do not dereference descriptor pointer outside of spinlock. Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> --- This is untested, respectively, never observed in life, but it seems to me, that while the driver avoids reading callback and callback_param outside of spinlock, same caution should be taken, when reading flags, but maybe I'm wrong, feel free to drop then. drivers/dma/ste_dma40.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)