[PATCH net] net: ethernet: nixge: Add missing check after DMA map

Thomas Fourier posted 1 patch 2 months, 1 week ago
drivers/net/ethernet/ni/nixge.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
[PATCH net] net: ethernet: nixge: Add missing check after DMA map
Posted by Thomas Fourier 2 months, 1 week ago
The DMA map functions can fail and should be tested for errors.

Fixes: 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev")
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
 drivers/net/ethernet/ni/nixge.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 230d5ff99dd7..027e53023007 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -334,6 +334,10 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
 		phys = dma_map_single(ndev->dev.parent, skb->data,
 				      NIXGE_MAX_JUMBO_FRAME_SIZE,
 				      DMA_FROM_DEVICE);
+		if (dma_mapping_error(ndev->dev.parent, phys)) {
+			dev_kfree_skb_any(skb);
+			goto out;
+		}
 
 		nixge_hw_dma_bd_set_phys(&priv->rx_bd_v[i], phys);
 
@@ -645,8 +649,8 @@ static int nixge_recv(struct net_device *ndev, int budget)
 					  NIXGE_MAX_JUMBO_FRAME_SIZE,
 					  DMA_FROM_DEVICE);
 		if (dma_mapping_error(ndev->dev.parent, cur_phys)) {
-			/* FIXME: bail out and clean up */
-			netdev_err(ndev, "Failed to map ...\n");
+			dev_kfree_skb_any(new_skb);
+			return packets;
 		}
 		nixge_hw_dma_bd_set_phys(cur_p, cur_phys);
 		cur_p->cntrl = NIXGE_MAX_JUMBO_FRAME_SIZE;
-- 
2.43.0
Re: [PATCH net] net: ethernet: nixge: Add missing check after DMA map
Posted by Eric Dumazet 2 months, 1 week ago
On Fri, Jul 25, 2025 at 6:34 AM Thomas Fourier <fourier.thomas@gmail.com> wrote:
>
> The DMA map functions can fail and should be tested for errors.
>
> Fixes: 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev")
> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
> ---
>  drivers/net/ethernet/ni/nixge.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
> index 230d5ff99dd7..027e53023007 100644
> --- a/drivers/net/ethernet/ni/nixge.c
> +++ b/drivers/net/ethernet/ni/nixge.c
> @@ -334,6 +334,10 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
>                 phys = dma_map_single(ndev->dev.parent, skb->data,
>                                       NIXGE_MAX_JUMBO_FRAME_SIZE,
>                                       DMA_FROM_DEVICE);
> +               if (dma_mapping_error(ndev->dev.parent, phys)) {
> +                       dev_kfree_skb_any(skb);
> +                       goto out;
> +               }
>
>                 nixge_hw_dma_bd_set_phys(&priv->rx_bd_v[i], phys);
>
> @@ -645,8 +649,8 @@ static int nixge_recv(struct net_device *ndev, int budget)
>                                           NIXGE_MAX_JUMBO_FRAME_SIZE,
>                                           DMA_FROM_DEVICE);
>                 if (dma_mapping_error(ndev->dev.parent, cur_phys)) {
> -                       /* FIXME: bail out and clean up */
> -                       netdev_err(ndev, "Failed to map ...\n");
> +                       dev_kfree_skb_any(new_skb);
> +                       return packets;

Note that this error (and the possible failed
netdev_alloc_skb_ip_align() at line 641) can leave the queue in a
frozen state,
because of a missing

nixge_dma_write_desc_reg(priv, XAXIDMA_RX_TDESC_OFFSET, tail_p);

Not sure if this driver is actively used...
Re: [PATCH net] net: ethernet: nixge: Add missing check after DMA map
Posted by Jakub Kicinski 2 months, 1 week ago
On Fri, 25 Jul 2025 06:53:16 -0700 Eric Dumazet wrote:
> Not sure if this driver is actively used...

Like most of the drivers that are missing dma_mappnig_error() :(

Thomas, would it be possible for you to sort the drivers you have
reports for in the reverse chronological order (when they were added
or last time they had significant work done on them)? Start from
the most recent ones?
Re: [PATCH net] net: ethernet: nixge: Add missing check after DMA map
Posted by Thomas Fourier 2 months, 1 week ago
On 25/07/2025 17:10, Jakub Kicinski wrote:
> On Fri, 25 Jul 2025 06:53:16 -0700 Eric Dumazet wrote:
>> Not sure if this driver is actively used...
> Like most of the drivers that are missing dma_mappnig_error() :(
>
> Thomas, would it be possible for you to sort the drivers you have
> reports for in the reverse chronological order (when they were added
> or last time they had significant work done on them)? Start from
> the most recent ones?

This is already one of the most recent drivers with missing 
dma_mapping_error(),

I have patched the missing dma_mapping_error from 2017 onward that I 
could find.

Here are the drivers that are missing a dma_ampping_error() that I could 
find sorted by

the last date in their copyright notice:

2017 drivers/net/ethernet/ni/nixge.c
2017 drivers/scsi/aacraid/commctrl.c
2013 drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c
2012 drivers/net/ethernet/rdc/r6040.c
2011 drivers/scsi/isci/request.c
2010 drivers/tty/serial/amba-pl011.c
2010 drivers/net/ethernet/neterion/s2io.c
2010 drivers/net/ethernet/micrel/ksz884x.c
2010 drivers/net/ethernet/marvell/pxa168_eth.c
2009 crypto/async_tx/async_pq.c
2008 drivers/message/fusion/mptlan.c
2008 drivers/message/fusion/mptctl.c
2008 drivers/atm/solos-pci.c
2007 sound/pci/sis7019.c
2007 drivers/scsi/initio.c
2007 drivers/scsi/esp_scsi.c
2007 drivers/net/ethernet/tehuti/tehuti.c
2007 drivers/media/pci/ivtv/ivtv-udma.c
2007 drivers/crypto/hifn_795x.c
2006 crypto/async_tx/async_xor.c
2006 crypto/async_tx/async_memcpy.c
2005 drivers/scsi/megaraid.c
2005 drivers/net/ethernet/chelsio/cxgb/sge.c
2005 drivers/net/ethernet/dec/tulip/uli526x.c
2004 drivers/net/ethernet/via/via-velocity.c
2004 drivers/net/ethernet/sun/sungem.c
2004 drivers/net/ethernet/marvell/mv643xx_eth.c
2003 drivers/tty/serial/atmel_serial.c
2003 drivers/scsi/ips.c
2003 drivers/scsi/dc395x.c
2003 drivers/net/wan/wanxl.c
2003 drivers/net/ethernet/sun/cassini.c
2002 drivers/net/hippi/rrunner.c
2002 drivers/net/ethernet/ti/tlan.c
2002 drivers/net/ethernet/alteon/acenic.c
2002 crypto/async_tx/async_raid6_recov.c
2001 drivers/scsi/53c700.c
2001 drivers/parport/parport_pc.c
2001 drivers/net/ethernet/smsc/epic100.c
2001 drivers/net/ethernet/packetengines/yellowfin.c
2001 drivers/net/ethernet/natsemi/ns83820.c
2001 drivers/net/ethernet/dlink/dl2k.c
2001 drivers/net/ethernet/dec/tulip/winbond-840.c
2001 drivers/net/ethernet/dec/tulip/tulip_core.c
2001 drivers/net/ethernet/dec/tulip/de2104x.c
2001 drivers/atm/he.c
2000 drivers/net/ethernet/toshiba/tc35815.c
2000 drivers/net/ethernet/packetengines/hamachi.c
2000 drivers/net/ethernet/fealnx.c
2000 drivers/net/ethernet/amd/amd8111e.c
2000 drivers/net/ethernet/3com/typhoon.c
1999 drivers/net/fddi/skfp/skfddi.c
1999 drivers/net/ethernet/3com/3c59x.c
1999 drivers/atm/nicstar.c
1998 drivers/net/wireless/intel/ipw2x00/ipw2200.c
1998 drivers/net/ethernet/dec/tulip/dmfe.c
1997 drivers/scsi/aha1740.c
Re: [PATCH net] net: ethernet: nixge: Add missing check after DMA map
Posted by Eric Dumazet 2 months, 1 week ago
On Fri, Jul 25, 2025 at 8:10 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 25 Jul 2025 06:53:16 -0700 Eric Dumazet wrote:
> > Not sure if this driver is actively used...
>
> Like most of the drivers that are missing dma_mappnig_error() :(

Well, a failure in __netdev_alloc_skb_ip_align() is more probable.

Maybe the following would help (and fix
ndev->stats.rx_packets/ndev->stats.rx_bytes) as well.

diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 230d5ff99dd7e1e9fabe21a6617d72d663cc1a7c..835463c301e11dca9f824137e75dd0eacf130419
100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -606,9 +606,6 @@ static int nixge_recv(struct net_device *ndev, int budget)

        while ((cur_p->status & XAXIDMA_BD_STS_COMPLETE_MASK &&
                budget > packets)) {
-               tail_p = priv->rx_bd_p + sizeof(*priv->rx_bd_v) *
-                        priv->rx_bd_ci;
-
                skb = (struct sk_buff *)(uintptr_t)
                        nixge_hw_dma_bd_get_addr(cur_p, sw_id_offset);

@@ -639,7 +636,7 @@ static int nixge_recv(struct net_device *ndev, int budget)
                new_skb = netdev_alloc_skb_ip_align(ndev,
                                                    NIXGE_MAX_JUMBO_FRAME_SIZE);
                if (!new_skb)
-                       return packets;
+                       goto end;

                cur_phys = dma_map_single(ndev->dev.parent, new_skb->data,
                                          NIXGE_MAX_JUMBO_FRAME_SIZE,
@@ -653,11 +650,15 @@ static int nixge_recv(struct net_device *ndev, int budget)
                cur_p->status = 0;
                nixge_hw_dma_bd_set_offset(cur_p, (uintptr_t)new_skb);

+               tail_p = priv->rx_bd_p + sizeof(*priv->rx_bd_v) *
+                        priv->rx_bd_ci;
+
                ++priv->rx_bd_ci;
                priv->rx_bd_ci %= RX_BD_NUM;
                cur_p = &priv->rx_bd_v[priv->rx_bd_ci];
        }

+end:
        ndev->stats.rx_packets += packets;
        ndev->stats.rx_bytes += size;