linux-kernel - Re: [PATCH] atmel_spi: Pass correct DMA address to controller

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]

Message-Id: <200705161254.59308.david-b@pacbell.net>
Date:	Wed, 16 May 2007 12:54:59 -0700
From:	David Brownell <david-b@...bell.net>
To:	Haavard Skinnemoen <hskinnemoen@...el.com>
Cc:	wux@...dicorp.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] atmel_spi: Pass correct DMA address to controller

On Wednesday 16 May 2007, Haavard Skinnemoen wrote:
> When either rx_buf or tx_buf is not being used, i.e. for plain read-
> or write operations, the atmel_spi uses a fixed-size DMA buffer
> instead. If the transfer is longer than the size of this buffer, it is
> split into multiple DMA transfers.
> 
> When the transfer is split like this, the atmel_spi driver ends up
> using the same DMA address again and again even for the buffer that
> came from the user, which is of course wrong.

Yes, and needs fixing.  This is almost right, see below.


> Fix this by adding the 
> number of bytes already transferred to the DMA address so that the
> data ends up in the right place.
> 
> Thanks to Wu Xuan for discovering this bug.
> 
> Signed-off-by: Haavard Skinnemoen <hskinnemoen@...el.com>
> ---
>  drivers/spi/atmel_spi.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
> index 1d8a2f6..8b2601d 100644
> --- a/drivers/spi/atmel_spi.c
> +++ b/drivers/spi/atmel_spi.c
> @@ -113,16 +113,16 @@ static void atmel_spi_next_xfer(struct spi_master *master,
>  
>  	len = as->remaining_bytes;
>  
> -	tx_dma = xfer->tx_dma;
> -	rx_dma = xfer->rx_dma;
> +	tx_dma = xfer->tx_dma + xfer->len - len;
> +	rx_dma = xfer->rx_dma + xfer->len - len;
>  
>  	/* use scratch buffer only when rx or tx data is unspecified */
> -	if (rx_dma == INVALID_DMA_ADDRESS) {
> +	if (!xfer->rx_buf) {

This test ...

>  		rx_dma = as->buffer_dma;
>  		if (len > BUFFER_SIZE)
>  			len = BUFFER_SIZE;
>  	}
> -	if (tx_dma == INVALID_DMA_ADDRESS) {
> +	if (!xfer->tx_buf) {

... and this one must not assume that the relevant buffer
is always NULL when the tx_dma has been set up.  Keep the
test against INVALID_DMA_ADDRESS, but fetch the address
from the spi_transfer.

It's legit to set up cpu-virtual (for PIO) and dma addresses
for each buffer, since the upper layer driver has no way to
know if the underlying controller driver is DMA-capable, or
for that matter PIO-capable.

>  		tx_dma = as->buffer_dma;
>  		if (len > BUFFER_SIZE)
>  			len = BUFFER_SIZE;
> -- 
> 1.4.4.4
> 


-
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/