netdev - Re: Validation of DMA params breaks e100 driver (2.6.28-rc2)

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-next>] [day] [month] [year] [list]

Message-ID: <20081031210131.GA18015@flint.arm.linux.org.uk>
Date:	Fri, 31 Oct 2008 21:01:32 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Anders Grafström 
	<grfstrm@...rs.sourceforge.net>
Cc:	linux-arm-kernel@...ts.arm.linux.org.uk, netdev@...r.kernel.org
Subject: Re: Validation of DMA params breaks e100 driver (2.6.28-rc2)

On Thu, Oct 30, 2008 at 05:27:59PM +0100, Anders Grafström wrote:
> The e100 driver triggers BUG_ON(buf->direction != dir)
> by doing pci_map_single(..., PCI_DMA_BIDIRECTIONAL)
> and pci_dma_sync_single_for_device(..., PCI_DMA_TODEVICE).
> 
> I'm guessing it's allowed to do that and that something like
> the patch below is called for?

No, it is not allowed to do that - that's why it's called "BUG_ON".
Changing the DMA direction, especially with dmabounce will result
in unexpected behaviour.

> -----------------
> 
> [PATCH] [ARM] dma: Fix DMA params validation
> 
> This patch makes the DMA params validation less strict
> and more like the generic implementation.
> 
> Signed-off-by: Anders Grafström <grfstrm@...rs.sourceforge.net>
> ---
>  arch/arm/common/dmabounce.c |   13 ++++++-------
>  1 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/common/dmabounce.c b/arch/arm/common/dmabounce.c
> index f030f07..a0c9c7b 100644
> --- a/arch/arm/common/dmabounce.c
> +++ b/arch/arm/common/dmabounce.c
> @@ -289,7 +289,6 @@ static inline void unmap_single(struct device *dev, dma_addr_t dma_addr,
> 
>  	if (buf) {
>  		BUG_ON(buf->size != size);
> -		BUG_ON(buf->direction != dir);
> 
>  		dev_dbg(dev,
>  			"%s: unsafe buffer %p (dma=%#x) mapped to %p (dma=%#x)\n",
> @@ -382,8 +381,6 @@ int dmabounce_sync_for_cpu(struct device *dev, dma_addr_t addr,
>  	if (!buf)
>  		return 1;
> 
> -	BUG_ON(buf->direction != dir);
> -
>  	dev_dbg(dev, "%s: unsafe buffer %p (dma=%#x) mapped to %p (dma=%#x)\n",
>  		__func__, buf->ptr, virt_to_dma(dev, buf->ptr),
>  		buf->safe, buf->safe_dma_addr);
> @@ -394,7 +391,9 @@ int dmabounce_sync_for_cpu(struct device *dev, dma_addr_t addr,
>  		dev_dbg(dev, "%s: copy back safe %p to unsafe %p size %d\n",
>  			__func__, buf->safe + off, buf->ptr + off, sz);
>  		memcpy(buf->ptr + off, buf->safe + off, sz);
> -	}
> +	} else
> +		BUG_ON(dir != DMA_TO_DEVICE);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(dmabounce_sync_for_cpu);
> @@ -411,8 +410,6 @@ int dmabounce_sync_for_device(struct device *dev, dma_addr_t addr,
>  	if (!buf)
>  		return 1;
> 
> -	BUG_ON(buf->direction != dir);
> -
>  	dev_dbg(dev, "%s: unsafe buffer %p (dma=%#x) mapped to %p (dma=%#x)\n",
>  		__func__, buf->ptr, virt_to_dma(dev, buf->ptr),
>  		buf->safe, buf->safe_dma_addr);
> @@ -423,7 +420,9 @@ int dmabounce_sync_for_device(struct device *dev, dma_addr_t addr,
>  		dev_dbg(dev, "%s: copy out unsafe %p to safe %p, size %d\n",
>  			__func__,buf->ptr + off, buf->safe + off, sz);
>  		memcpy(buf->safe + off, buf->ptr + off, sz);
> -	}
> +	} else
> +		BUG_ON(dir != DMA_FROM_DEVICE);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(dmabounce_sync_for_device);
> -- 
> 1.5.6.5
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html