The driver does not check if mapping dma memory succeed.
The patch adds the checks and failure handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <[email protected]>
---
drivers/net/wan/dscc4.c | 52 +++++++++++++++++++++++++++++++++++--------------
1 file changed, 37 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
index 799830ffcae2..1a94f0a95b2c 100644
--- a/drivers/net/wan/dscc4.c
+++ b/drivers/net/wan/dscc4.c
@@ -522,19 +522,27 @@ static inline int try_get_rx_skb(struct dscc4_dev_priv *dpriv,
struct RxFD *rx_fd = dpriv->rx_fd + dirty;
const int len = RX_MAX(HDLC_MAX_MRU);
struct sk_buff *skb;
- int ret = 0;
+ dma_addr_t addr;
skb = dev_alloc_skb(len);
dpriv->rx_skbuff[dirty] = skb;
- if (skb) {
- skb->protocol = hdlc_type_trans(skb, dev);
- rx_fd->data = cpu_to_le32(pci_map_single(dpriv->pci_priv->pdev,
- skb->data, len, PCI_DMA_FROMDEVICE));
- } else {
- rx_fd->data = 0;
- ret = -1;
- }
- return ret;
+ if (!skb)
+ goto err_out;
+
+ skb->protocol = hdlc_type_trans(skb, dev);
+ addr = pci_map_single(dpriv->pci_priv->pdev,
+ skb->data, len, PCI_DMA_FROMDEVICE);
+ if (pci_dma_mapping_error(dpriv->pci_priv->pdev, addr))
+ goto err_free_skb;
+
+ rx_fd->data = cpu_to_le32(addr);
+ return 0;
+
+err_free_skb:
+ dev_kfree_skb_any(skb);
+err_out:
+ rx_fd->data = 0;
+ return -1;
}
/*
@@ -1147,14 +1155,22 @@ static netdev_tx_t dscc4_start_xmit(struct sk_buff *skb,
struct dscc4_dev_priv *dpriv = dscc4_priv(dev);
struct dscc4_pci_priv *ppriv = dpriv->pci_priv;
struct TxFD *tx_fd;
+ dma_addr_t addr;
int next;
+ addr = pci_map_single(ppriv->pdev, skb->data, skb->len,
+ PCI_DMA_TODEVICE);
+ if (pci_dma_mapping_error(ppriv->pdev, addr)) {
+ dev_kfree_skb_any(skb);
+ dev->stats.tx_errors++;
+ return NETDEV_TX_OK;
+ }
+
next = dpriv->tx_current%TX_RING_SIZE;
dpriv->tx_skbuff[next] = skb;
tx_fd = dpriv->tx_fd + next;
tx_fd->state = FrameEnd | TO_STATE_TX(skb->len);
- tx_fd->data = cpu_to_le32(pci_map_single(ppriv->pdev, skb->data, skb->len,
- PCI_DMA_TODEVICE));
+ tx_fd->data = cpu_to_le32(addr);
tx_fd->complete = 0x00000000;
tx_fd->jiffies = jiffies;
mb();
@@ -1889,14 +1905,20 @@ static struct sk_buff *dscc4_init_dummy_skb(struct dscc4_dev_priv *dpriv)
if (skb) {
int last = dpriv->tx_dirty%TX_RING_SIZE;
struct TxFD *tx_fd = dpriv->tx_fd + last;
+ dma_addr_t addr;
skb->len = DUMMY_SKB_SIZE;
skb_copy_to_linear_data(skb, version,
strlen(version) % DUMMY_SKB_SIZE);
tx_fd->state = FrameEnd | TO_STATE_TX(DUMMY_SKB_SIZE);
- tx_fd->data = cpu_to_le32(pci_map_single(dpriv->pci_priv->pdev,
- skb->data, DUMMY_SKB_SIZE,
- PCI_DMA_TODEVICE));
+ addr = pci_map_single(dpriv->pci_priv->pdev,
+ skb->data, DUMMY_SKB_SIZE,
+ PCI_DMA_TODEVICE);
+ if (pci_dma_mapping_error(dpriv->pci_priv->pdev, addr)) {
+ dev_kfree_skb_any(skb);
+ return NULL;
+ }
+ tx_fd->data = cpu_to_le32(addr);
dpriv->tx_skbuff[last] = skb;
}
return skb;
--
2.7.4
From: Alexey Khoroshilov <[email protected]>
Date: Fri, 4 Aug 2017 23:23:24 +0300
> The driver does not check if mapping dma memory succeed.
> The patch adds the checks and failure handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <[email protected]>
This is a great example of why it can be irritating to see these
mechanical "bug fixes" for drivers very few people use and actually
test, which introduces new bugs.
> @@ -522,19 +522,27 @@ static inline int try_get_rx_skb(struct dscc4_dev_priv *dpriv,
> struct RxFD *rx_fd = dpriv->rx_fd + dirty;
> const int len = RX_MAX(HDLC_MAX_MRU);
> struct sk_buff *skb;
> - int ret = 0;
> + dma_addr_t addr;
>
> skb = dev_alloc_skb(len);
> dpriv->rx_skbuff[dirty] = skb;
skb recorded here.
> +err_free_skb:
> + dev_kfree_skb_any(skb);
Yet freed here in the error path.
dpriv->rx_skbuff[dirty] should not be set to 'skb' until all possibile
failure tests have passed.
Alexey Khoroshilov <[email protected]> :
> The driver does not check if mapping dma memory succeed.
> The patch adds the checks and failure handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <[email protected]>
Please amend your subject line as:
Subject: [PATCH net-next v2 1/1] dscc4: add checks for dma mapping errors.
Rationale: davem is not supposed to guess the branch the patch should be
applied to.
[...]
> diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
> index 799830ffcae2..1a94f0a95b2c 100644
> --- a/drivers/net/wan/dscc4.c
> +++ b/drivers/net/wan/dscc4.c
> @@ -522,19 +522,27 @@ static inline int try_get_rx_skb(struct dscc4_dev_priv *dpriv,
> struct RxFD *rx_fd = dpriv->rx_fd + dirty;
> const int len = RX_MAX(HDLC_MAX_MRU);
> struct sk_buff *skb;
> - int ret = 0;
> + dma_addr_t addr;
>
> skb = dev_alloc_skb(len);
> dpriv->rx_skbuff[dirty] = skb;
> - if (skb) {
> - skb->protocol = hdlc_type_trans(skb, dev);
> - rx_fd->data = cpu_to_le32(pci_map_single(dpriv->pci_priv->pdev,
> - skb->data, len, PCI_DMA_FROMDEVICE));
> - } else {
> - rx_fd->data = 0;
> - ret = -1;
> - }
> - return ret;
> + if (!skb)
> + goto err_out;
> +
> + skb->protocol = hdlc_type_trans(skb, dev);
> + addr = pci_map_single(dpriv->pci_priv->pdev,
> + skb->data, len, PCI_DMA_FROMDEVICE);
> + if (pci_dma_mapping_error(dpriv->pci_priv->pdev, addr))
> + goto err_free_skb;
Nit: please use a local 'struct pci_dev *pdev = dpriv->pci_priv->pdev;'
[...]
> @@ -1147,14 +1155,22 @@ static netdev_tx_t dscc4_start_xmit(struct sk_buff *skb,
> struct dscc4_dev_priv *dpriv = dscc4_priv(dev);
> struct dscc4_pci_priv *ppriv = dpriv->pci_priv;
> struct TxFD *tx_fd;
> + dma_addr_t addr;
> int next;
>
> + addr = pci_map_single(ppriv->pdev, skb->data, skb->len,
> + PCI_DMA_TODEVICE);
> + if (pci_dma_mapping_error(ppriv->pdev, addr)) {
> + dev_kfree_skb_any(skb);
> + dev->stats.tx_errors++;
It should read 'dev->stats.tx_dropped++'.
--
Ueimor
The driver does not check if mapping dma memory succeed.
The patch adds the checks and failure handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <[email protected]>
---
v2: Fix issues noted by David Miller and Francois Romieu.
drivers/net/wan/dscc4.c | 52 +++++++++++++++++++++++++++++++++++--------------
1 file changed, 37 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
index 799830f..6a9ffac 100644
--- a/drivers/net/wan/dscc4.c
+++ b/drivers/net/wan/dscc4.c
@@ -518,23 +518,31 @@ static void dscc4_release_ring(struct dscc4_dev_priv *dpriv)
static inline int try_get_rx_skb(struct dscc4_dev_priv *dpriv,
struct net_device *dev)
{
+ struct pci_dev *pdev = dpriv->pci_priv->pdev;
unsigned int dirty = dpriv->rx_dirty%RX_RING_SIZE;
struct RxFD *rx_fd = dpriv->rx_fd + dirty;
const int len = RX_MAX(HDLC_MAX_MRU);
struct sk_buff *skb;
- int ret = 0;
+ dma_addr_t addr;
skb = dev_alloc_skb(len);
+ if (!skb)
+ goto err_out;
+
+ skb->protocol = hdlc_type_trans(skb, dev);
+ addr = pci_map_single(pdev, skb->data, len, PCI_DMA_FROMDEVICE);
+ if (pci_dma_mapping_error(pdev, addr))
+ goto err_free_skb;
+
dpriv->rx_skbuff[dirty] = skb;
- if (skb) {
- skb->protocol = hdlc_type_trans(skb, dev);
- rx_fd->data = cpu_to_le32(pci_map_single(dpriv->pci_priv->pdev,
- skb->data, len, PCI_DMA_FROMDEVICE));
- } else {
- rx_fd->data = 0;
- ret = -1;
- }
- return ret;
+ rx_fd->data = cpu_to_le32(addr);
+ return 0;
+
+err_free_skb:
+ dev_kfree_skb_any(skb);
+err_out:
+ rx_fd->data = 0;
+ return -1;
}
/*
@@ -1147,14 +1155,22 @@ static netdev_tx_t dscc4_start_xmit(struct sk_buff *skb,
struct dscc4_dev_priv *dpriv = dscc4_priv(dev);
struct dscc4_pci_priv *ppriv = dpriv->pci_priv;
struct TxFD *tx_fd;
+ dma_addr_t addr;
int next;
+ addr = pci_map_single(ppriv->pdev, skb->data, skb->len,
+ PCI_DMA_TODEVICE);
+ if (pci_dma_mapping_error(ppriv->pdev, addr)) {
+ dev_kfree_skb_any(skb);
+ dev->stats.tx_dropped++;
+ return NETDEV_TX_OK;
+ }
+
next = dpriv->tx_current%TX_RING_SIZE;
dpriv->tx_skbuff[next] = skb;
tx_fd = dpriv->tx_fd + next;
tx_fd->state = FrameEnd | TO_STATE_TX(skb->len);
- tx_fd->data = cpu_to_le32(pci_map_single(ppriv->pdev, skb->data, skb->len,
- PCI_DMA_TODEVICE));
+ tx_fd->data = cpu_to_le32(addr);
tx_fd->complete = 0x00000000;
tx_fd->jiffies = jiffies;
mb();
@@ -1887,16 +1903,22 @@ static struct sk_buff *dscc4_init_dummy_skb(struct dscc4_dev_priv *dpriv)
skb = dev_alloc_skb(DUMMY_SKB_SIZE);
if (skb) {
+ struct pci_dev *pdev = dpriv->pci_priv->pdev;
int last = dpriv->tx_dirty%TX_RING_SIZE;
struct TxFD *tx_fd = dpriv->tx_fd + last;
+ dma_addr_t addr;
skb->len = DUMMY_SKB_SIZE;
skb_copy_to_linear_data(skb, version,
strlen(version) % DUMMY_SKB_SIZE);
tx_fd->state = FrameEnd | TO_STATE_TX(DUMMY_SKB_SIZE);
- tx_fd->data = cpu_to_le32(pci_map_single(dpriv->pci_priv->pdev,
- skb->data, DUMMY_SKB_SIZE,
- PCI_DMA_TODEVICE));
+ addr = pci_map_single(pdev, skb->data, DUMMY_SKB_SIZE,
+ PCI_DMA_TODEVICE);
+ if (pci_dma_mapping_error(pdev, addr)) {
+ dev_kfree_skb_any(skb);
+ return NULL;
+ }
+ tx_fd->data = cpu_to_le32(addr);
dpriv->tx_skbuff[last] = skb;
}
return skb;
--
2.7.4
Alexey Khoroshilov <[email protected]> :
[...]
> diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
> index 799830f..6a9ffac 100644
> --- a/drivers/net/wan/dscc4.c
> +++ b/drivers/net/wan/dscc4.c
> @@ -518,23 +518,31 @@ static void dscc4_release_ring(struct dscc4_dev_priv *dpriv)
> static inline int try_get_rx_skb(struct dscc4_dev_priv *dpriv,
> struct net_device *dev)
> {
> + struct pci_dev *pdev = dpriv->pci_priv->pdev;
> unsigned int dirty = dpriv->rx_dirty%RX_RING_SIZE;
> struct RxFD *rx_fd = dpriv->rx_fd + dirty;
> const int len = RX_MAX(HDLC_MAX_MRU);
For the edification of the masses, you may follow a strict inverted
xmas tree style (aka longer lines first as long as it does not bug).
[...]
> @@ -1147,14 +1155,22 @@ static netdev_tx_t dscc4_start_xmit(struct sk_buff *skb,
> struct dscc4_dev_priv *dpriv = dscc4_priv(dev);
> struct dscc4_pci_priv *ppriv = dpriv->pci_priv;
> struct TxFD *tx_fd;
> + dma_addr_t addr;
> int next;
>
> + addr = pci_map_single(ppriv->pdev, skb->data, skb->len,
> + PCI_DMA_TODEVICE);
Use a local struct pci_dev *pdev and it fits on a single line.
At some point it will probably be converted to plain dma api and use a 'd' dev.
[...]
> @@ -1887,16 +1903,22 @@ static struct sk_buff *dscc4_init_dummy_skb(struct dscc4_dev_priv *dpriv)
>
> skb = dev_alloc_skb(DUMMY_SKB_SIZE);
> if (skb) {
> + struct pci_dev *pdev = dpriv->pci_priv->pdev;
> int last = dpriv->tx_dirty%TX_RING_SIZE;
> struct TxFD *tx_fd = dpriv->tx_fd + last;
> + dma_addr_t addr;
>
> skb->len = DUMMY_SKB_SIZE;
> skb_copy_to_linear_data(skb, version,
> strlen(version) % DUMMY_SKB_SIZE);
> tx_fd->state = FrameEnd | TO_STATE_TX(DUMMY_SKB_SIZE);
> - tx_fd->data = cpu_to_le32(pci_map_single(dpriv->pci_priv->pdev,
> - skb->data, DUMMY_SKB_SIZE,
> - PCI_DMA_TODEVICE));
> + addr = pci_map_single(pdev, skb->data, DUMMY_SKB_SIZE,
> + PCI_DMA_TODEVICE);
> + if (pci_dma_mapping_error(pdev, addr)) {
> + dev_kfree_skb_any(skb);
> + return NULL;
> + }
> + tx_fd->data = cpu_to_le32(addr);
> dpriv->tx_skbuff[last] = skb;
> }
> return skb;
It isn't technically wrong but please don't update tx_fd before the mapping
succeeds. It will look marginally better.
Thanks.
--
Ueimor
On Tue, Aug 8, 2017 at 10:28 PM, Alexey Khoroshilov
<[email protected]> wrote:
> The driver does not check if mapping dma memory succeed.
> The patch adds the checks and failure handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
Since it is going to be v3, just to mention that IIRC we better not to
use PCI DMA mapping API in favour of generic DMA mapping API.
Can you please double check this?
--
With Best Regards,
Andy Shevchenko
The driver does not check if mapping dma memory succeed.
The patch adds the checks and failure handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <[email protected]>
---
v2: Fix issues noted by David Miller and Francois Romieu.
v3: Improve code per Francois Romieu recommendations.
Convert to plain DMA API.
drivers/net/wan/dscc4.c | 53 ++++++++++++++++++++++++++++++++++---------------
1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
index 799830f..8480dbf 100644
--- a/drivers/net/wan/dscc4.c
+++ b/drivers/net/wan/dscc4.c
@@ -519,22 +519,30 @@ static inline int try_get_rx_skb(struct dscc4_dev_priv *dpriv,
struct net_device *dev)
{
unsigned int dirty = dpriv->rx_dirty%RX_RING_SIZE;
+ struct pci_dev *pdev = dpriv->pci_priv->pdev;
struct RxFD *rx_fd = dpriv->rx_fd + dirty;
const int len = RX_MAX(HDLC_MAX_MRU);
struct sk_buff *skb;
- int ret = 0;
+ dma_addr_t addr;
skb = dev_alloc_skb(len);
+ if (!skb)
+ goto err_out;
+
+ skb->protocol = hdlc_type_trans(skb, dev);
+ addr = pci_map_single(pdev, skb->data, len, PCI_DMA_FROMDEVICE);
+ if (pci_dma_mapping_error(pdev, addr))
+ goto err_free_skb;
+
dpriv->rx_skbuff[dirty] = skb;
- if (skb) {
- skb->protocol = hdlc_type_trans(skb, dev);
- rx_fd->data = cpu_to_le32(pci_map_single(dpriv->pci_priv->pdev,
- skb->data, len, PCI_DMA_FROMDEVICE));
- } else {
- rx_fd->data = 0;
- ret = -1;
- }
- return ret;
+ rx_fd->data = cpu_to_le32(addr);
+ return 0;
+
+err_free_skb:
+ dev_kfree_skb_any(skb);
+err_out:
+ rx_fd->data = 0;
+ return -1;
}
/*
@@ -1145,16 +1153,23 @@ static netdev_tx_t dscc4_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
struct dscc4_dev_priv *dpriv = dscc4_priv(dev);
- struct dscc4_pci_priv *ppriv = dpriv->pci_priv;
+ struct pci_dev *pdev = dpriv->pci_priv->pdev;
struct TxFD *tx_fd;
+ dma_addr_t addr;
int next;
+ addr = pci_map_single(pdev, skb->data, skb->len, PCI_DMA_TODEVICE);
+ if (pci_dma_mapping_error(pdev, addr)) {
+ dev_kfree_skb_any(skb);
+ dev->stats.tx_dropped++;
+ return NETDEV_TX_OK;
+ }
+
next = dpriv->tx_current%TX_RING_SIZE;
dpriv->tx_skbuff[next] = skb;
tx_fd = dpriv->tx_fd + next;
tx_fd->state = FrameEnd | TO_STATE_TX(skb->len);
- tx_fd->data = cpu_to_le32(pci_map_single(ppriv->pdev, skb->data, skb->len,
- PCI_DMA_TODEVICE));
+ tx_fd->data = cpu_to_le32(addr);
tx_fd->complete = 0x00000000;
tx_fd->jiffies = jiffies;
mb();
@@ -1887,16 +1902,22 @@ static struct sk_buff *dscc4_init_dummy_skb(struct dscc4_dev_priv *dpriv)
skb = dev_alloc_skb(DUMMY_SKB_SIZE);
if (skb) {
+ struct pci_dev *pdev = dpriv->pci_priv->pdev;
int last = dpriv->tx_dirty%TX_RING_SIZE;
struct TxFD *tx_fd = dpriv->tx_fd + last;
+ dma_addr_t addr;
skb->len = DUMMY_SKB_SIZE;
skb_copy_to_linear_data(skb, version,
strlen(version) % DUMMY_SKB_SIZE);
+ addr = pci_map_single(pdev, skb->data, DUMMY_SKB_SIZE,
+ PCI_DMA_TODEVICE);
+ if (pci_dma_mapping_error(pdev, addr)) {
+ dev_kfree_skb_any(skb);
+ return NULL;
+ }
tx_fd->state = FrameEnd | TO_STATE_TX(DUMMY_SKB_SIZE);
- tx_fd->data = cpu_to_le32(pci_map_single(dpriv->pci_priv->pdev,
- skb->data, DUMMY_SKB_SIZE,
- PCI_DMA_TODEVICE));
+ tx_fd->data = cpu_to_le32(addr);
dpriv->tx_skbuff[last] = skb;
}
return skb;
--
2.7.4
Make use the dma_*() interfaces rather than the pci_*() interfaces.
Signed-off-by: Alexey Khoroshilov <[email protected]>
---
drivers/net/wan/dscc4.c | 96 ++++++++++++++++++++++++++-----------------------
1 file changed, 51 insertions(+), 45 deletions(-)
diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
index 8480dbf..a043fb1 100644
--- a/drivers/net/wan/dscc4.c
+++ b/drivers/net/wan/dscc4.c
@@ -483,20 +483,20 @@ static void dscc4_tx_print(struct net_device *dev,
static void dscc4_release_ring(struct dscc4_dev_priv *dpriv)
{
- struct pci_dev *pdev = dpriv->pci_priv->pdev;
+ struct device *d = &dpriv->pci_priv->pdev->dev;
struct TxFD *tx_fd = dpriv->tx_fd;
struct RxFD *rx_fd = dpriv->rx_fd;
struct sk_buff **skbuff;
int i;
- pci_free_consistent(pdev, TX_TOTAL_SIZE, tx_fd, dpriv->tx_fd_dma);
- pci_free_consistent(pdev, RX_TOTAL_SIZE, rx_fd, dpriv->rx_fd_dma);
+ dma_free_coherent(d, TX_TOTAL_SIZE, tx_fd, dpriv->tx_fd_dma);
+ dma_free_coherent(d, RX_TOTAL_SIZE, rx_fd, dpriv->rx_fd_dma);
skbuff = dpriv->tx_skbuff;
for (i = 0; i < TX_RING_SIZE; i++) {
if (*skbuff) {
- pci_unmap_single(pdev, le32_to_cpu(tx_fd->data),
- (*skbuff)->len, PCI_DMA_TODEVICE);
+ dma_unmap_single(d, le32_to_cpu(tx_fd->data),
+ (*skbuff)->len, DMA_TO_DEVICE);
dev_kfree_skb(*skbuff);
}
skbuff++;
@@ -506,8 +506,9 @@ static void dscc4_release_ring(struct dscc4_dev_priv *dpriv)
skbuff = dpriv->rx_skbuff;
for (i = 0; i < RX_RING_SIZE; i++) {
if (*skbuff) {
- pci_unmap_single(pdev, le32_to_cpu(rx_fd->data),
- RX_MAX(HDLC_MAX_MRU), PCI_DMA_FROMDEVICE);
+ dma_unmap_single(d, le32_to_cpu(rx_fd->data),
+ RX_MAX(HDLC_MAX_MRU),
+ DMA_FROM_DEVICE);
dev_kfree_skb(*skbuff);
}
skbuff++;
@@ -519,7 +520,7 @@ static inline int try_get_rx_skb(struct dscc4_dev_priv *dpriv,
struct net_device *dev)
{
unsigned int dirty = dpriv->rx_dirty%RX_RING_SIZE;
- struct pci_dev *pdev = dpriv->pci_priv->pdev;
+ struct device *d = &dpriv->pci_priv->pdev->dev;
struct RxFD *rx_fd = dpriv->rx_fd + dirty;
const int len = RX_MAX(HDLC_MAX_MRU);
struct sk_buff *skb;
@@ -530,8 +531,8 @@ static inline int try_get_rx_skb(struct dscc4_dev_priv *dpriv,
goto err_out;
skb->protocol = hdlc_type_trans(skb, dev);
- addr = pci_map_single(pdev, skb->data, len, PCI_DMA_FROMDEVICE);
- if (pci_dma_mapping_error(pdev, addr))
+ addr = dma_map_single(d, skb->data, len, DMA_FROM_DEVICE);
+ if (dma_mapping_error(d, addr))
goto err_free_skb;
dpriv->rx_skbuff[dirty] = skb;
@@ -654,7 +655,7 @@ static inline void dscc4_rx_skb(struct dscc4_dev_priv *dpriv,
struct net_device *dev)
{
struct RxFD *rx_fd = dpriv->rx_fd + dpriv->rx_current%RX_RING_SIZE;
- struct pci_dev *pdev = dpriv->pci_priv->pdev;
+ struct device *d = &dpriv->pci_priv->pdev->dev;
struct sk_buff *skb;
int pkt_len;
@@ -664,8 +665,8 @@ static inline void dscc4_rx_skb(struct dscc4_dev_priv *dpriv,
goto refill;
}
pkt_len = TO_SIZE(le32_to_cpu(rx_fd->state2));
- pci_unmap_single(pdev, le32_to_cpu(rx_fd->data),
- RX_MAX(HDLC_MAX_MRU), PCI_DMA_FROMDEVICE);
+ dma_unmap_single(d, le32_to_cpu(rx_fd->data),
+ RX_MAX(HDLC_MAX_MRU), DMA_FROM_DEVICE);
if ((skb->data[--pkt_len] & FrameOk) == FrameOk) {
dev->stats.rx_packets++;
dev->stats.rx_bytes += pkt_len;
@@ -782,8 +783,8 @@ static int dscc4_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
rc = -ENOMEM;
- priv->iqcfg = (__le32 *) pci_alloc_consistent(pdev,
- IRQ_RING_SIZE*sizeof(__le32), &priv->iqcfg_dma);
+ priv->iqcfg = (__le32 *)dma_alloc_coherent(&pdev->dev,
+ IRQ_RING_SIZE*sizeof(__le32), &priv->iqcfg_dma, GFP_KERNEL);
if (!priv->iqcfg)
goto err_free_irq_5;
writel(priv->iqcfg_dma, ioaddr + IQCFG);
@@ -794,16 +795,18 @@ static int dscc4_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
*/
for (i = 0; i < dev_per_card; i++) {
dpriv = priv->root + i;
- dpriv->iqtx = (__le32 *) pci_alloc_consistent(pdev,
- IRQ_RING_SIZE*sizeof(u32), &dpriv->iqtx_dma);
+ dpriv->iqtx = (__le32 *)dma_alloc_coherent(&pdev->dev,
+ IRQ_RING_SIZE*sizeof(u32), &dpriv->iqtx_dma,
+ GFP_KERNEL);
if (!dpriv->iqtx)
goto err_free_iqtx_6;
writel(dpriv->iqtx_dma, ioaddr + IQTX0 + i*4);
}
for (i = 0; i < dev_per_card; i++) {
dpriv = priv->root + i;
- dpriv->iqrx = (__le32 *) pci_alloc_consistent(pdev,
- IRQ_RING_SIZE*sizeof(u32), &dpriv->iqrx_dma);
+ dpriv->iqrx = (__le32 *)dma_alloc_coherent(&pdev->dev,
+ IRQ_RING_SIZE*sizeof(u32), &dpriv->iqrx_dma,
+ GFP_KERNEL);
if (!dpriv->iqrx)
goto err_free_iqrx_7;
writel(dpriv->iqrx_dma, ioaddr + IQRX0 + i*4);
@@ -827,18 +830,18 @@ static int dscc4_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
err_free_iqrx_7:
while (--i >= 0) {
dpriv = priv->root + i;
- pci_free_consistent(pdev, IRQ_RING_SIZE*sizeof(u32),
- dpriv->iqrx, dpriv->iqrx_dma);
+ dma_free_coherent(&pdev->dev, IRQ_RING_SIZE*sizeof(u32),
+ dpriv->iqrx, dpriv->iqrx_dma);
}
i = dev_per_card;
err_free_iqtx_6:
while (--i >= 0) {
dpriv = priv->root + i;
- pci_free_consistent(pdev, IRQ_RING_SIZE*sizeof(u32),
- dpriv->iqtx, dpriv->iqtx_dma);
+ dma_free_coherent(&pdev->dev, IRQ_RING_SIZE*sizeof(u32),
+ dpriv->iqtx, dpriv->iqtx_dma);
}
- pci_free_consistent(pdev, IRQ_RING_SIZE*sizeof(u32), priv->iqcfg,
- priv->iqcfg_dma);
+ dma_free_coherent(&pdev->dev, IRQ_RING_SIZE*sizeof(u32), priv->iqcfg,
+ priv->iqcfg_dma);
err_free_irq_5:
free_irq(pdev->irq, priv->root);
err_release_4:
@@ -1153,13 +1156,13 @@ static netdev_tx_t dscc4_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
struct dscc4_dev_priv *dpriv = dscc4_priv(dev);
- struct pci_dev *pdev = dpriv->pci_priv->pdev;
+ struct device *d = &dpriv->pci_priv->pdev->dev;
struct TxFD *tx_fd;
dma_addr_t addr;
int next;
- addr = pci_map_single(pdev, skb->data, skb->len, PCI_DMA_TODEVICE);
- if (pci_dma_mapping_error(pdev, addr)) {
+ addr = dma_map_single(d, skb->data, skb->len, DMA_TO_DEVICE);
+ if (dma_mapping_error(d, addr)) {
dev_kfree_skb_any(skb);
dev->stats.tx_dropped++;
return NETDEV_TX_OK;
@@ -1587,8 +1590,9 @@ static void dscc4_tx_irq(struct dscc4_pci_priv *ppriv,
tx_fd = dpriv->tx_fd + cur;
skb = dpriv->tx_skbuff[cur];
if (skb) {
- pci_unmap_single(ppriv->pdev, le32_to_cpu(tx_fd->data),
- skb->len, PCI_DMA_TODEVICE);
+ dma_unmap_single(&ppriv->pdev->dev,
+ le32_to_cpu(tx_fd->data),
+ skb->len, DMA_TO_DEVICE);
if (tx_fd->state & FrameEnd) {
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;
@@ -1902,7 +1906,7 @@ static struct sk_buff *dscc4_init_dummy_skb(struct dscc4_dev_priv *dpriv)
skb = dev_alloc_skb(DUMMY_SKB_SIZE);
if (skb) {
- struct pci_dev *pdev = dpriv->pci_priv->pdev;
+ struct device *d = &dpriv->pci_priv->pdev->dev;
int last = dpriv->tx_dirty%TX_RING_SIZE;
struct TxFD *tx_fd = dpriv->tx_fd + last;
dma_addr_t addr;
@@ -1910,9 +1914,9 @@ static struct sk_buff *dscc4_init_dummy_skb(struct dscc4_dev_priv *dpriv)
skb->len = DUMMY_SKB_SIZE;
skb_copy_to_linear_data(skb, version,
strlen(version) % DUMMY_SKB_SIZE);
- addr = pci_map_single(pdev, skb->data, DUMMY_SKB_SIZE,
- PCI_DMA_TODEVICE);
- if (pci_dma_mapping_error(pdev, addr)) {
+ addr = dma_map_single(d, skb->data, DUMMY_SKB_SIZE,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(d, addr)) {
dev_kfree_skb_any(skb);
return NULL;
}
@@ -1926,18 +1930,20 @@ static struct sk_buff *dscc4_init_dummy_skb(struct dscc4_dev_priv *dpriv)
static int dscc4_init_ring(struct net_device *dev)
{
struct dscc4_dev_priv *dpriv = dscc4_priv(dev);
- struct pci_dev *pdev = dpriv->pci_priv->pdev;
+ struct device *d = &dpriv->pci_priv->pdev->dev;
struct TxFD *tx_fd;
struct RxFD *rx_fd;
void *ring;
int i;
- ring = pci_alloc_consistent(pdev, RX_TOTAL_SIZE, &dpriv->rx_fd_dma);
+ ring = dma_alloc_coherent(d, RX_TOTAL_SIZE, &dpriv->rx_fd_dma,
+ GFP_KERNEL);
if (!ring)
goto err_out;
dpriv->rx_fd = rx_fd = (struct RxFD *) ring;
- ring = pci_alloc_consistent(pdev, TX_TOTAL_SIZE, &dpriv->tx_fd_dma);
+ ring = dma_alloc_coherent(d, TX_TOTAL_SIZE, &dpriv->tx_fd_dma,
+ GFP_KERNEL);
if (!ring)
goto err_free_dma_rx;
dpriv->tx_fd = tx_fd = (struct TxFD *) ring;
@@ -1975,9 +1981,9 @@ static int dscc4_init_ring(struct net_device *dev)
return 0;
err_free_dma_tx:
- pci_free_consistent(pdev, TX_TOTAL_SIZE, ring, dpriv->tx_fd_dma);
+ dma_free_coherent(d, TX_TOTAL_SIZE, ring, dpriv->tx_fd_dma);
err_free_dma_rx:
- pci_free_consistent(pdev, RX_TOTAL_SIZE, rx_fd, dpriv->rx_fd_dma);
+ dma_free_coherent(d, RX_TOTAL_SIZE, rx_fd, dpriv->rx_fd_dma);
err_out:
return -ENOMEM;
}
@@ -1997,16 +2003,16 @@ static void dscc4_remove_one(struct pci_dev *pdev)
dscc4_pci_reset(pdev, ioaddr);
free_irq(pdev->irq, root);
- pci_free_consistent(pdev, IRQ_RING_SIZE*sizeof(u32), ppriv->iqcfg,
- ppriv->iqcfg_dma);
+ dma_free_coherent(&pdev->dev, IRQ_RING_SIZE*sizeof(u32), ppriv->iqcfg,
+ ppriv->iqcfg_dma);
for (i = 0; i < dev_per_card; i++) {
struct dscc4_dev_priv *dpriv = root + i;
dscc4_release_ring(dpriv);
- pci_free_consistent(pdev, IRQ_RING_SIZE*sizeof(u32),
- dpriv->iqrx, dpriv->iqrx_dma);
- pci_free_consistent(pdev, IRQ_RING_SIZE*sizeof(u32),
- dpriv->iqtx, dpriv->iqtx_dma);
+ dma_free_coherent(&pdev->dev, IRQ_RING_SIZE*sizeof(u32),
+ dpriv->iqrx, dpriv->iqrx_dma);
+ dma_free_coherent(&pdev->dev, IRQ_RING_SIZE*sizeof(u32),
+ dpriv->iqtx, dpriv->iqtx_dma);
}
dscc4_free1(pdev);
--
2.7.4
From: Alexey Khoroshilov <[email protected]>
Date: Fri, 11 Aug 2017 01:55:20 +0300
> The driver does not check if mapping dma memory succeed.
> The patch adds the checks and failure handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <[email protected]>
Applied.
From: Alexey Khoroshilov <[email protected]>
Date: Fri, 11 Aug 2017 01:55:21 +0300
> Make use the dma_*() interfaces rather than the pci_*() interfaces.
>
> Signed-off-by: Alexey Khoroshilov <[email protected]>
Applied.
Alexey Khoroshilov <[email protected]> :
[...]
> diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
> index 8480dbf..a043fb1 100644
> --- a/drivers/net/wan/dscc4.c
> +++ b/drivers/net/wan/dscc4.c
[...]
> @@ -506,8 +506,9 @@ static void dscc4_release_ring(struct dscc4_dev_priv *dpriv)
> skbuff = dpriv->rx_skbuff;
> for (i = 0; i < RX_RING_SIZE; i++) {
> if (*skbuff) {
> - pci_unmap_single(pdev, le32_to_cpu(rx_fd->data),
> - RX_MAX(HDLC_MAX_MRU), PCI_DMA_FROMDEVICE);
> + dma_unmap_single(d, le32_to_cpu(rx_fd->data),
> + RX_MAX(HDLC_MAX_MRU),
> + DMA_FROM_DEVICE);
RX_MAX(HDLC_MAX_MRU), DMA_FROM_DEVICE);
[...]
> @@ -664,8 +665,8 @@ static inline void dscc4_rx_skb(struct dscc4_dev_priv *dpriv,
> goto refill;
> }
> pkt_len = TO_SIZE(le32_to_cpu(rx_fd->state2));
> - pci_unmap_single(pdev, le32_to_cpu(rx_fd->data),
> - RX_MAX(HDLC_MAX_MRU), PCI_DMA_FROMDEVICE);
> + dma_unmap_single(d, le32_to_cpu(rx_fd->data),
> + RX_MAX(HDLC_MAX_MRU), DMA_FROM_DEVICE);
dma_unmap_single(d, le32_to_cpu(rx_fd->data), RX_MAX(HDLC_MAX_MRU),
DMA_FROM_DEVICE);
[...]
> @@ -782,8 +783,8 @@ static int dscc4_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> rc = -ENOMEM;
>
> - priv->iqcfg = (__le32 *) pci_alloc_consistent(pdev,
> - IRQ_RING_SIZE*sizeof(__le32), &priv->iqcfg_dma);
> + priv->iqcfg = (__le32 *)dma_alloc_coherent(&pdev->dev,
> + IRQ_RING_SIZE*sizeof(__le32), &priv->iqcfg_dma, GFP_KERNEL);
- the cast can go away
- please replace &pdev->dev with a local variable
priv->iqcfg = dma_alloc_coherent(d, IRQ_RING_SIZE*sizeof(__le32),
&priv->iqcfg_dma, GFP_KERNEL);
Same thing for iqtx and iqrx (beware of copy&paste + tx/rx mismatch).
Thanks.
--
Ueimor
From: Francois Romieu <[email protected]>
Date: Fri, 11 Aug 2017 23:45:49 +0200
> Alexey Khoroshilov <[email protected]> :
> [...]
>> diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
>> index 8480dbf..a043fb1 100644
>> --- a/drivers/net/wan/dscc4.c
>> +++ b/drivers/net/wan/dscc4.c
> [...]
>> @@ -506,8 +506,9 @@ static void dscc4_release_ring(struct dscc4_dev_priv *dpriv)
>> skbuff = dpriv->rx_skbuff;
>> for (i = 0; i < RX_RING_SIZE; i++) {
>> if (*skbuff) {
>> - pci_unmap_single(pdev, le32_to_cpu(rx_fd->data),
>> - RX_MAX(HDLC_MAX_MRU), PCI_DMA_FROMDEVICE);
>> + dma_unmap_single(d, le32_to_cpu(rx_fd->data),
>> + RX_MAX(HDLC_MAX_MRU),
>> + DMA_FROM_DEVICE);
>
> RX_MAX(HDLC_MAX_MRU), DMA_FROM_DEVICE);
...
>> @@ -782,8 +783,8 @@ static int dscc4_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>
>> rc = -ENOMEM;
>>
>> - priv->iqcfg = (__le32 *) pci_alloc_consistent(pdev,
>> - IRQ_RING_SIZE*sizeof(__le32), &priv->iqcfg_dma);
>> + priv->iqcfg = (__le32 *)dma_alloc_coherent(&pdev->dev,
>> + IRQ_RING_SIZE*sizeof(__le32), &priv->iqcfg_dma, GFP_KERNEL);
>
> - the cast can go away
> - please replace &pdev->dev with a local variable
>
> priv->iqcfg = dma_alloc_coherent(d, IRQ_RING_SIZE*sizeof(__le32),
> &priv->iqcfg_dma, GFP_KERNEL);
>
> Same thing for iqtx and iqrx (beware of copy&paste + tx/rx mismatch).
Oops, this will need to be sent as a relative fixup as I've applied these
two patches to net-next, sorry Francois.
David Miller <[email protected]> :
[...]
> Oops, this will need to be sent as a relative fixup as I've applied these
> two patches to net-next, sorry Francois.
No problem. It works perfectly this way.
--
Ueimor