linux-kernel - RE: [PATCH 2/2] dma/ep93xx_dma: prevent ep93xx_dma

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, 22 Nov 2011 14:26:52 -0600
From:	H Hartley Sweeten <hartleys@...ionengravers.com>
To:	Mika Westerberg <mika.westerberg@....fi>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	"vinod.koul@...el.com" <vinod.koul@...el.com>,
	"dan.j.williams@...el.com" <dan.j.williams@...el.com>,
	"rmallon@...il.com" <rmallon@...il.com>,
	Rafal Prylowski <prylowski@...asoft.pl>
Subject: RE: [PATCH 2/2] dma/ep93xx_dma: prevent ep93xx_dma_tasklet() to
 access empty list

On Tuesday, November 22, 2011 12:46 PM, Mika Westerberg wrote:
> From: Rafal Prylowski <prylowski@...asoft.pl>
>
> If dma_terminate_all() is called before the ep93xx_dma_tasklet() gets to run,
> it tries to access an empty ->active list causing following OOPS:

[snip]

> As we expect that the ->active list is never empty when the ep93xx_dma_tasklet()
> is called, we fix this by adding a new flag per channel EP93XX_DMA_IS_RUNNING
> which determines whether the channel is running or not.
> 
> We also add BUG_ON() to ep93xx_dma_get_active() to make sure that similar
> problems will be caught early.
> 
> Signed-off-by: Rafal Prylowski <prylowski@...asoft.pl>
> [added a flag instead of just checking for empty list]
> Signed-off-by: Mika Westerberg <mika.westerberg@....fi>
> ---
>  drivers/dma/ep93xx_dma.c |   25 ++++++++++++++++---------
>  1 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
> index 6181811..cc3a302 100644
> --- a/drivers/dma/ep93xx_dma.c
> +++ b/drivers/dma/ep93xx_dma.c
> @@ -155,6 +155,8 @@ struct ep93xx_dma_chan {
>  	unsigned long			flags;
>  /* Channel is configured for cyclic transfers */
>  #define EP93XX_DMA_IS_CYCLIC		0
> +/* Channel is enabled */
> +#define EP93XX_DMA_IS_RUNNING		1
>  
>  	int				buffer;
>  	dma_cookie_t			last_completed;
> @@ -246,6 +248,7 @@ static void ep93xx_dma_set_active(struct ep93xx_dma_chan *edmac,
>  static struct ep93xx_dma_desc *
>  ep93xx_dma_get_active(struct ep93xx_dma_chan *edmac)
>  {
> +	BUG_ON(list_empty(&edmac->active));
>  	return list_first_entry(&edmac->active, struct ep93xx_dma_desc, node);

Mika,

Thanks for looking into this.

I still don't like the BUG_ON here.  Is it even possible to get here with
an empty list now that your catching it in the tasklet?

But, ep93xx_dma_set_active() also has a BUG_ON test so...

Acked-by: H Hartley Sweeten <hsweeten@...ionengravers.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/