| 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
| ||
|
Message-ID: <Pine.WNT.4.63.0811131718390.984@jbrandeb-desk.amr.corp.intel.com>
Date: Thu, 13 Nov 2008 17:21:50 -0800 (Pacific Standard Time)
From: "Brandeburg, Jesse" <jesse.brandeburg@...el.com>
To: Russell King - ARM Linux <linux@....linux.org.uk>
cc: Anders Grafström <grfstrm@...rs.sourceforge.net>,
"linux-arm-kernel@...ts.arm.linux.org.uk"
<linux-arm-kernel@...ts.arm.linux.org.uk>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
e1000-devel@...ts.sourceforge.net
Subject: Re: Validation of DMA params breaks e100 driver (2.6.28-rc2)
On Fri, 31 Oct 2008, Russell King - ARM Linux wrote:
> 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.
okay, how about this patch... only compile tested as I couldn't get
net-next-2.6 to boot on my test machine. I'll get some testing done on
this, but in the meantime....
e100: fix dma error in direction for mapping
From: Jesse Brandeburg <jesse.brandeburg@...el.com>
Russell King - ARM Linux wrote:
> 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.
This is my attempt to fix this issue:
Reported by: Anders Grafstrom <grfstrm@...rs.sourceforge.net>
CC: Russell King <linux@....linux.org.uk>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
---
drivers/net/e100.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index 62cdefa..c9c7079 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -166,7 +166,7 @@
#define DRV_NAME "e100"
#define DRV_EXT "-NAPI"
-#define DRV_VERSION "3.5.23-k4"DRV_EXT
+#define DRV_VERSION "3.5.23-k6"DRV_EXT
#define DRV_DESCRIPTION "Intel(R) PRO/100 Network Driver"
#define DRV_COPYRIGHT "Copyright(c) 1999-2006 Intel Corporation"
#define PFX DRV_NAME ": "
@@ -1804,7 +1804,7 @@ static int e100_rx_alloc_skb(struct nic *nic, struct rx *rx)
struct rfd *prev_rfd = (struct rfd *)rx->prev->skb->data;
put_unaligned_le32(rx->dma_addr, &prev_rfd->link);
pci_dma_sync_single_for_device(nic->pdev, rx->prev->dma_addr,
- sizeof(struct rfd), PCI_DMA_TODEVICE);
+ sizeof(struct rfd), PCI_DMA_BIDIRECTIONAL);
}
return 0;
@@ -1823,7 +1823,7 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx,
/* Need to sync before taking a peek at cb_complete bit */
pci_dma_sync_single_for_cpu(nic->pdev, rx->dma_addr,
- sizeof(struct rfd), PCI_DMA_FROMDEVICE);
+ sizeof(struct rfd), PCI_DMA_BIDIRECTIONAL);
rfd_status = le16_to_cpu(rfd->status);
DPRINTK(RX_STATUS, DEBUG, "status=0x%04X\n", rfd_status);
@@ -1850,7 +1850,7 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx,
/* Get data */
pci_unmap_single(nic->pdev, rx->dma_addr,
- RFD_BUF_LEN, PCI_DMA_FROMDEVICE);
+ RFD_BUF_LEN, PCI_DMA_BIDIRECTIONAL);
/* If this buffer has the el bit, but we think the receiver
* is still running, check to see if it really stopped while
@@ -1942,7 +1942,7 @@ static void e100_rx_clean(struct nic *nic, unsigned int *work_done,
new_before_last_rfd->command |= cpu_to_le16(cb_el);
pci_dma_sync_single_for_device(nic->pdev,
new_before_last_rx->dma_addr, sizeof(struct rfd),
- PCI_DMA_TODEVICE);
+ PCI_DMA_BIDIRECTIONAL);
/* Now that we have a new stopping point, we can clear the old
* stopping point. We must sync twice to get the proper
@@ -1950,11 +1950,11 @@ static void e100_rx_clean(struct nic *nic, unsigned int *work_done,
old_before_last_rfd->command &= ~cpu_to_le16(cb_el);
pci_dma_sync_single_for_device(nic->pdev,
old_before_last_rx->dma_addr, sizeof(struct rfd),
- PCI_DMA_TODEVICE);
+ PCI_DMA_BIDIRECTIONAL);
old_before_last_rfd->size = cpu_to_le16(VLAN_ETH_FRAME_LEN);
pci_dma_sync_single_for_device(nic->pdev,
old_before_last_rx->dma_addr, sizeof(struct rfd),
- PCI_DMA_TODEVICE);
+ PCI_DMA_BIDIRECTIONAL);
}
if(restart_required) {
@@ -1977,7 +1977,7 @@ static void e100_rx_clean_list(struct nic *nic)
for(rx = nic->rxs, i = 0; i < count; rx++, i++) {
if(rx->skb) {
pci_unmap_single(nic->pdev, rx->dma_addr,
- RFD_BUF_LEN, PCI_DMA_FROMDEVICE);
+ RFD_BUF_LEN, PCI_DMA_BIDIRECTIONAL);
dev_kfree_skb(rx->skb);
}
}
@@ -2020,7 +2020,7 @@ static int e100_rx_alloc_list(struct nic *nic)
before_last->command |= cpu_to_le16(cb_el);
before_last->size = 0;
pci_dma_sync_single_for_device(nic->pdev, rx->dma_addr,
- sizeof(struct rfd), PCI_DMA_TODEVICE);
+ sizeof(struct rfd), PCI_DMA_BIDIRECTIONAL);
nic->rx_to_use = nic->rx_to_clean = nic->rxs;
nic->ru_running = RU_SUSPENDED;
@@ -2221,7 +2221,7 @@ static int e100_loopback_test(struct nic *nic, enum loopback loopback_mode)
msleep(10);
pci_dma_sync_single_for_cpu(nic->pdev, nic->rx_to_clean->dma_addr,
- RFD_BUF_LEN, PCI_DMA_FROMDEVICE);
+ RFD_BUF_LEN, PCI_DMA_BIDIRECTIONAL);
if(memcmp(nic->rx_to_clean->skb->data + sizeof(struct rfd),
skb->data, ETH_DATA_LEN))
Powered by blists - more mailing lists