[PATCH net v2] net: dlink: handle dma_map_single() failure properly

[PATCH net v2] net: dlink: handle dma_map_single() failure properly

[Yeounsu Moon]

There is no error handling for `dma_map_single()` failures.

Add error handling by checking `dma_mapping_error()` and freeing
the `skb` using `dev_kfree_skb()` (process context) when it fails.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Yeounsu Moon <yyyynoom@gmail.com>
Tested-on: D-Link DGE-550T Rev-A3
Suggested-by: Simon Horman <horms@kernel.org>
---
Changelog:
v2:
- fix one thing properly
- use goto statement, per Simon's suggestion
v1: https://lore.kernel.org/netdev/20251002152638.1165-1-yyyynoom@gmail.com/
---
 drivers/net/ethernet/dlink/dl2k.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
index 1996d2e4e3e2..7077d705e471 100644
--- a/drivers/net/ethernet/dlink/dl2k.c
+++ b/drivers/net/ethernet/dlink/dl2k.c
@@ -508,25 +508,34 @@ static int alloc_list(struct net_device *dev)
 	for (i = 0; i < RX_RING_SIZE; i++) {
 		/* Allocated fixed size of skbuff */
 		struct sk_buff *skb;
+		dma_addr_t addr;
 
 		skb = netdev_alloc_skb_ip_align(dev, np->rx_buf_sz);
 		np->rx_skbuff[i] = skb;
-		if (!skb) {
-			free_list(dev);
-			return -ENOMEM;
-		}
+		if (!skb)
+			goto err_free_list;
+
+		addr = dma_map_single(&np->pdev->dev, skb->data,
+				      np->rx_buf_sz, DMA_FROM_DEVICE);
+		if (dma_mapping_error(&np->pdev->dev, addr))
+			goto err_kfree_skb;
 
 		np->rx_ring[i].next_desc = cpu_to_le64(np->rx_ring_dma +
 						((i + 1) % RX_RING_SIZE) *
 						sizeof(struct netdev_desc));
 		/* Rubicon now supports 40 bits of addressing space. */
-		np->rx_ring[i].fraginfo =
-		    cpu_to_le64(dma_map_single(&np->pdev->dev, skb->data,
-					       np->rx_buf_sz, DMA_FROM_DEVICE));
+		np->rx_ring[i].fraginfo = cpu_to_le64(addr);
 		np->rx_ring[i].fraginfo |= cpu_to_le64((u64)np->rx_buf_sz << 48);
 	}
 
 	return 0;
+
+err_kfree_skb:
+	dev_kfree_skb(np->rx_skbuff[i]);
+	np->rx_skbuff[i] = NULL;
+err_free_list:
+	free_list(dev);
+	return -ENOMEM;
 }
 
 static void rio_hw_init(struct net_device *dev)
-- 
2.51.0

Re: [PATCH net v2] net: dlink: handle dma_map_single() failure properly

[Simon Horman]

On Fri, Oct 10, 2025 at 12:57:16AM +0900, Yeounsu Moon wrote:
> There is no error handling for `dma_map_single()` failures.
> 
> Add error handling by checking `dma_mapping_error()` and freeing
> the `skb` using `dev_kfree_skb()` (process context) when it fails.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Yeounsu Moon <yyyynoom@gmail.com>
> Tested-on: D-Link DGE-550T Rev-A3
> Suggested-by: Simon Horman <horms@kernel.org>

FWIIW, I don't think my Suggested-by tag is strictly necessary here. I did
suggest an implementation approach. And I'm very happy that you took my
idea on board. But I'd view as more of a tweak in this case. Because the
overall meaning of the patch remains the same as your original version.

> ---
> Changelog:
> v2:
> - fix one thing properly
> - use goto statement, per Simon's suggestion
> v1: https://lore.kernel.org/netdev/20251002152638.1165-1-yyyynoom@gmail.com/

Thanks for the update, this version looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v2] net: dlink: handle dma_map_single() failure properly

[Yeounsu Moon]

On Fri Oct 10, 2025 at 3:55 PM KST, Simon Horman wrote:
> On Fri, Oct 10, 2025 at 12:57:16AM +0900, Yeounsu Moon wrote:
>> There is no error handling for `dma_map_single()` failures.
>> 
>> Add error handling by checking `dma_mapping_error()` and freeing
>> the `skb` using `dev_kfree_skb()` (process context) when it fails.
>> 
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Yeounsu Moon <yyyynoom@gmail.com>
>> Tested-on: D-Link DGE-550T Rev-A3
>> Suggested-by: Simon Horman <horms@kernel.org>
>
> FWIIW, I don't think my Suggested-by tag is strictly necessary here. I did
> suggest an implementation approach. And I'm very happy that you took my
> idea on board. But I'd view as more of a tweak in this case. Because the
> overall meaning of the patch remains the same as your original version.
>
Thank you for letting me know,
>> ---
>> Changelog:
>> v2:
>> - fix one thing properly
>> - use goto statement, per Simon's suggestion
>> v1: https://lore.kernel.org/netdev/20251002152638.1165-1-yyyynoom@gmail.com/
>
> Thanks for the update, this version looks good to me.
>
> Reviewed-by: Simon Horman <horms@kernel.org>
And I really appreciate your review.

Re: [PATCH net v2] net: dlink: handle dma_map_single() failure properly

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 10 Oct 2025 00:57:16 +0900 you wrote:
> There is no error handling for `dma_map_single()` failures.
> 
> Add error handling by checking `dma_mapping_error()` and freeing
> the `skb` using `dev_kfree_skb()` (process context) when it fails.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Yeounsu Moon <yyyynoom@gmail.com>
> Tested-on: D-Link DGE-550T Rev-A3
> Suggested-by: Simon Horman <horms@kernel.org>
> 
> [...]

Here is the summary with links:
  - [net,v2] net: dlink: handle dma_map_single() failure properly
    https://git.kernel.org/netdev/net/c/65946eac6d88

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html