[PATCH net-next v2 0/2] net: stmmac: DMA address handling fixes

[PATCH net-next v2 0/2] net: stmmac: DMA address handling fixes

[Alex Elder]

Update jumbo_frm() (implemented in both "chain_mode.c" and "ring_mode.c")
to ensure DMA errors don't get lost due to casting the result to an
integer of the wrong size.  This was a Sashiko suggestion.

In addition, add explicit calls to lower_32_bits() to DMA addresses
before they are passed to cpu_to_le32(), to guarantee the type of
the converted value is u32.

Maxime implied that a new version the first patch could handle both
fixes, but I thought it better to do them separately.
 
					-Alex

Here is v1 of the series (the patch):
  https://lore.kernel.org/lkml/20260811192703.1765416-1-elder@riscstar.com/	 

Alex Elder (2):
  net: stmmac: use dma_addr_t for DMA addresses
  net: stmmac: convert DMA address to lower 32 before assignment

 drivers/net/ethernet/stmicro/stmmac/chain_mode.c |  9 +++++----
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c  | 15 ++++++++-------
 2 files changed, 13 insertions(+), 11 deletions(-)


base-commit: 5e6de6a2b522f659defacb1551d0465ba6ce13cf
-- 
2.53.0

[PATCH net-next v2 1/2] net: stmmac: use dma_addr_t for DMA addresses

[Alex Elder]

In jumbo_frm() (implemented in both "chain_mode.c" and "ring_mode.c"),
an unsigned integer local variable is used to hold the value returned
by dma_map_single().  On systems where a dma_addr_t is 64 bits, the
subsequent dma_mapping_error() check of the returned value operates
only on the low 32 bits (whose high bit won't be sign-extended).  In
this case, dma_mapping_error() would return 0 (no error) even if there
were one.

Fix this in both spots by using a dma_addr_t for the local variable.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-devicetree/20260606010122.21A211F00899@smtp.kernel.org/
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 drivers/net/ethernet/stmicro/stmmac/chain_mode.c | 3 ++-
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
index fc04a23342cfc..ec25193d287bb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
@@ -20,9 +20,10 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
 	unsigned int nopaged_len = skb_headlen(skb);
 	struct stmmac_priv *priv = tx_q->priv_data;
 	unsigned int entry = tx_q->cur_tx;
-	unsigned int bmax, buf_len, des2;
+	unsigned int bmax, buf_len;
 	unsigned int i = 1, len;
 	struct dma_desc *desc;
+	dma_addr_t des2;
 
 	desc = tx_q->dma_tx + entry;
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
index 78fc6aa5bbe95..664d8cfb58cdc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
@@ -20,8 +20,9 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
 	unsigned int nopaged_len = skb_headlen(skb);
 	struct stmmac_priv *priv = tx_q->priv_data;
 	unsigned int entry = tx_q->cur_tx;
-	unsigned int bmax, len, des2;
+	unsigned int bmax, len;
 	struct dma_desc *desc;
+	dma_addr_t des2;
 
 	if (priv->extend_desc)
 		desc = (struct dma_desc *)(tx_q->dma_etx + entry);
-- 
2.53.0

[PATCH net-next v2 2/2] net: stmmac: convert DMA address to lower 32 before assignment

[Alex Elder]

In jumbo_frm() (implemented in both "chain_mode.c" and "ring_mode.c"),
there are places where a DMA descriptor is converted to little-endian
byte order in assignment.  The DMA descriptor could be a 64-bit value,
which makes the 32-bit byte swapping operation seem a little sketchy.

Explicitly extract the low-order 32 bits of the dma_addr_t value being
converted into a u32 so it's crystal clear that we're doing the right
thing.

Suggested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 drivers/net/ethernet/stmicro/stmmac/chain_mode.c |  6 +++---
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c  | 12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
index ec25193d287bb..66025e2509e91 100644
--- a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
@@ -37,7 +37,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
 
 	des2 = dma_map_single(priv->device, skb->data,
 			      buf_len, DMA_TO_DEVICE);
-	desc->des2 = cpu_to_le32(des2);
+	desc->des2 = cpu_to_le32(lower_32_bits(des2));
 	if (dma_mapping_error(priv->device, des2))
 		return -1;
 	tx_q->tx_skbuff_dma[entry].buf = des2;
@@ -55,7 +55,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
 			des2 = dma_map_single(priv->device,
 					      (skb->data + bmax * i),
 					      bmax, DMA_TO_DEVICE);
-			desc->des2 = cpu_to_le32(des2);
+			desc->des2 = cpu_to_le32(lower_32_bits(des2));
 			if (dma_mapping_error(priv->device, des2))
 				return -1;
 			tx_q->tx_skbuff_dma[entry].buf = des2;
@@ -68,7 +68,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
 			des2 = dma_map_single(priv->device,
 					      (skb->data + bmax * i), len,
 					      DMA_TO_DEVICE);
-			desc->des2 = cpu_to_le32(des2);
+			desc->des2 = cpu_to_le32(lower_32_bits(des2));
 			if (dma_mapping_error(priv->device, des2))
 				return -1;
 			tx_q->tx_skbuff_dma[entry].buf = des2;
diff --git a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
index 664d8cfb58cdc..f7949419eb9fd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
@@ -40,7 +40,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
 
 		des2 = dma_map_single(priv->device, skb->data, bmax,
 				      DMA_TO_DEVICE);
-		desc->des2 = cpu_to_le32(des2);
+		desc->des2 = cpu_to_le32(lower_32_bits(des2));
 		if (dma_mapping_error(priv->device, des2))
 			return -1;
 
@@ -48,7 +48,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
 		tx_q->tx_skbuff_dma[entry].len = bmax;
 		tx_q->tx_skbuff_dma[entry].is_jumbo = true;
 
-		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
+		desc->des3 = cpu_to_le32(lower_32_bits(des2) + BUF_SIZE_4KiB);
 		stmmac_prepare_tx_desc(priv, desc, 1, bmax, csum,
 				STMMAC_RING_MODE, 0, false, skb->len);
 		tx_q->tx_skbuff[entry] = NULL;
@@ -61,27 +61,27 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
 
 		des2 = dma_map_single(priv->device, skb->data + bmax, len,
 				      DMA_TO_DEVICE);
-		desc->des2 = cpu_to_le32(des2);
+		desc->des2 = cpu_to_le32(lower_32_bits(des2));
 		if (dma_mapping_error(priv->device, des2))
 			return -1;
 		tx_q->tx_skbuff_dma[entry].buf = des2;
 		tx_q->tx_skbuff_dma[entry].len = len;
 		tx_q->tx_skbuff_dma[entry].is_jumbo = true;
 
-		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
+		desc->des3 = cpu_to_le32(lower_32_bits(des2) + BUF_SIZE_4KiB);
 		stmmac_prepare_tx_desc(priv, desc, 0, len, csum,
 				STMMAC_RING_MODE, 1, !skb_is_nonlinear(skb),
 				skb->len);
 	} else {
 		des2 = dma_map_single(priv->device, skb->data,
 				      nopaged_len, DMA_TO_DEVICE);
-		desc->des2 = cpu_to_le32(des2);
+		desc->des2 = cpu_to_le32(lower_32_bits(des2));
 		if (dma_mapping_error(priv->device, des2))
 			return -1;
 		tx_q->tx_skbuff_dma[entry].buf = des2;
 		tx_q->tx_skbuff_dma[entry].len = nopaged_len;
 		tx_q->tx_skbuff_dma[entry].is_jumbo = true;
-		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
+		desc->des3 = cpu_to_le32(lower_32_bits(des2) + BUF_SIZE_4KiB);
 		stmmac_prepare_tx_desc(priv, desc, 1, nopaged_len, csum,
 				STMMAC_RING_MODE, 0, !skb_is_nonlinear(skb),
 				skb->len);
-- 
2.53.0