[PATCH net 0/2] net: systemport and bcmgenet OOM fixes

[PATCH net 0/2] net: systemport and bcmgenet OOM fixes

[Florian Fainelli]

Hi David,

These two patches fix similar Out of Memory code paths in the SYSTEMPORT and
GENET drivers. Under high memory pressure, we could produce an OOPS by
passing a NULL pointer to dma_unmap_single().

Thanks!

Florian Fainelli (2):
  net: systemport: check harder for out of memory conditions
  net: bcmgenet: check harder for out of memory conditions

 drivers/net/ethernet/broadcom/bcmsysport.c     | 31 ++++++++++++++----------
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 33 +++++++++++++++-----------
 2 files changed, 38 insertions(+), 26 deletions(-)

-- 
1.9.1

[PATCH net 1/2] net: systemport: check harder for out of memory conditions

[Florian Fainelli]

There is a potential case where we might be failing to refill a
control block, leaving it with both a NULL skb pointer *and* a NULL
dma_unmap_addr.

The way we process incoming packets, by first calling
dma_unmap_single(), and then only checking for a potential NULL skb can
lead to situations where do pass a NULL dma_unmap_addr() to
dma_unmap_single(), resulting in an oops.

Fix this my moving the NULL skb check earlier, since no backing skb
also means no corresponding DMA mapping for this packet.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 31 ++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 6f4e18644bd4..d9b9170ed2fc 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -534,6 +534,25 @@ static unsigned int bcm_sysport_desc_rx(struct bcm_sysport_priv *priv,
 	while ((processed < to_process) && (processed < budget)) {
 		cb = &priv->rx_cbs[priv->rx_read_ptr];
 		skb = cb->skb;
+
+		processed++;
+		priv->rx_read_ptr++;
+
+		if (priv->rx_read_ptr == priv->num_rx_bds)
+			priv->rx_read_ptr = 0;
+
+		/* We do not have a backing SKB, so we do not a corresponding
+		 * DMA mapping for this incoming packet since
+		 * bcm_sysport_rx_refill always either has both skb and mapping
+		 * or none.
+		 */
+		if (unlikely(!skb)) {
+			netif_err(priv, rx_err, ndev, "out of memory!\n");
+			ndev->stats.rx_dropped++;
+			ndev->stats.rx_errors++;
+			goto refill;
+		}
+
 		dma_unmap_single(kdev, dma_unmap_addr(cb, dma_addr),
 				 RX_BUF_LENGTH, DMA_FROM_DEVICE);
 
@@ -543,23 +562,11 @@ static unsigned int bcm_sysport_desc_rx(struct bcm_sysport_priv *priv,
 		status = (rsb->rx_status_len >> DESC_STATUS_SHIFT) &
 			  DESC_STATUS_MASK;
 
-		processed++;
-		priv->rx_read_ptr++;
-		if (priv->rx_read_ptr == priv->num_rx_bds)
-			priv->rx_read_ptr = 0;
-
 		netif_dbg(priv, rx_status, ndev,
 			  "p=%d, c=%d, rd_ptr=%d, len=%d, flag=0x%04x\n",
 			  p_index, priv->rx_c_index, priv->rx_read_ptr,
 			  len, status);
 
-		if (unlikely(!skb)) {
-			netif_err(priv, rx_err, ndev, "out of memory!\n");
-			ndev->stats.rx_dropped++;
-			ndev->stats.rx_errors++;
-			goto refill;
-		}
-
 		if (unlikely(!(status & DESC_EOP) || !(status & DESC_SOP))) {
 			netif_err(priv, rx_status, ndev, "fragmented packet!\n");
 			ndev->stats.rx_dropped++;
-- 
1.9.1

[PATCH net 2/2] net: bcmgenet: check harder for out of memory conditions

[Florian Fainelli]

There is a potential case where we might be failing to refill a
control block, leaving it with both a NULL skb pointer *and* a NULL
dma_unmap_addr.

The way we process incoming packets, by first calling
dma_unmap_single(), and then only checking for a potential NULL skb can
lead to situations where do pass a NULL dma_unmap_addr() to
dma_unmap_single(), resulting in an oops.

Fix this my moving the NULL skb check earlier, since no backing skb
also means no corresponding DMA mapping for this packet.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 33 +++++++++++++++-----------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 3f9d4de8173c..cdef86a03862 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1274,12 +1274,29 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_priv *priv,
 
 	while ((rxpktprocessed < rxpkttoprocess) &&
 	       (rxpktprocessed < budget)) {
+		cb = &priv->rx_cbs[priv->rx_read_ptr];
+		skb = cb->skb;
+
+		rxpktprocessed++;
+
+		priv->rx_read_ptr++;
+		priv->rx_read_ptr &= (priv->num_rx_bds - 1);
+
+		/* We do not have a backing SKB, so we do not have a
+		 * corresponding DMA mapping for this incoming packet since
+		 * bcmgenet_rx_refill always either has both skb and mapping or
+		 * none.
+		 */
+		if (unlikely(!skb)) {
+			dev->stats.rx_dropped++;
+			dev->stats.rx_errors++;
+			goto refill;
+		}
+
 		/* Unmap the packet contents such that we can use the
 		 * RSV from the 64 bytes descriptor when enabled and save
 		 * a 32-bits register read
 		 */
-		cb = &priv->rx_cbs[priv->rx_read_ptr];
-		skb = cb->skb;
 		dma_unmap_single(&dev->dev, dma_unmap_addr(cb, dma_addr),
 				 priv->rx_buf_len, DMA_FROM_DEVICE);
 
@@ -1307,18 +1324,6 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_priv *priv,
 			  __func__, p_index, priv->rx_c_index,
 			  priv->rx_read_ptr, dma_length_status);
 
-		rxpktprocessed++;
-
-		priv->rx_read_ptr++;
-		priv->rx_read_ptr &= (priv->num_rx_bds - 1);
-
-		/* out of memory, just drop packets at the hardware level */
-		if (unlikely(!skb)) {
-			dev->stats.rx_dropped++;
-			dev->stats.rx_errors++;
-			goto refill;
-		}
-
 		if (unlikely(!(dma_flag & DMA_EOP) || !(dma_flag & DMA_SOP))) {
 			netif_err(priv, rx_status, dev,
 				  "dropping fragmented packet!\n");
-- 
1.9.1

Re: [PATCH net 0/2] net: systemport and bcmgenet OOM fixes

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon,  8 Sep 2014 11:37:50 -0700

> These two patches fix similar Out of Memory code paths in the SYSTEMPORT and
> GENET drivers. Under high memory pressure, we could produce an OOPS by
> passing a NULL pointer to dma_unmap_single().

Series applied, thanks Florian.