[PATCH net 0/2] net: bcmasp: fix TX ring accounting bugs

[PATCH net 0/2] net: bcmasp: fix TX ring accounting bugs

[Danesh Petigara]

Two fixes for TX descriptor ring handling in the bcmasp driver:

  - tx_spb_ring_full() re-initialized next_index from
    intf->tx_spb_index on every loop iteration instead of advancing
    it, so it only ever checked a single descriptor slot regardless
    of cnt. This let bcmasp_xmit() proceed even when the ring didn't
    actually have enough free slots for the SKB's fragments.

  - bcmasp_xmit() only set txcb->last for the final fragment of an
    SKB, leaving stale true values in reused descriptor slots from a
    prior transmission. Combined with the ring-full miscount above,
    this could cause bcmasp_tx_reclaim() to treat a mid-SKB
    descriptor as the last one and free the sk_buff while later
    fragments were still in flight.

Patch 1 clears txcb->last unconditionally before it is set, and
patch 2 fixes the ring-full slot check to advance through each
candidate slot.

Justin Chen (2):
  net: bcmasp: clear txcb->last before writing each descriptor
  net: bcmasp: fix tx_spb_ring_full() checking same slot cnt times

 drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.54.0

[PATCH net 1/2] net: bcmasp: clear txcb->last before writing each descriptor

[Danesh Petigara]

From: Justin Chen <justin.chen@broadcom.com>

bcmasp_xmit() only wrote txcb->last = true for the final fragment
of an SKB; non-final fragments left the field untouched.  If a
descriptor slot was reused while it still held a stale true from
a previous SKB (possible when tx_spb_ring_full() underreported
fullness), bcmasp_tx_reclaim() would see last == true mid-SKB and
call dev_consume_skb_any() prematurely, freeing the sk_buff while
its remaining fragments were still in flight.

Unconditionally clear txcb->last before the conditional set so every
descriptor slot starts from a known false state regardless of what a
prior transmission left behind.

Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller")
Signed-off-by: Justin Chen <justin.chen@broadcom.com>
Assisted-by: Claude:claude-sonnet-4-6 vscode
Signed-off-by: Danesh Petigara <danesh.petigara@broadcom.com>
---
 drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c
index ed0977832ce4..2bd035f74fa2 100644
--- a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c
+++ b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c
@@ -301,6 +301,7 @@ static netdev_tx_t bcmasp_xmit(struct sk_buff *skb, struct net_device *dev)
 		txcb->bytes_sent = total_bytes;
 		dma_unmap_addr_set(txcb, dma_addr, mapping);
 		dma_unmap_len_set(txcb, dma_len, size);
+		txcb->last = false;
 		if (!i) {
 			desc->flags |= DESC_SOF;
 			if (csum_hw)
-- 
2.54.0

Re: [PATCH net 1/2] net: bcmasp: clear txcb->last before writing each descriptor

[Florian Fainelli]

On 8/31/26 11:42, Danesh Petigara wrote:
> From: Justin Chen <justin.chen@broadcom.com>
> 
> bcmasp_xmit() only wrote txcb->last = true for the final fragment
> of an SKB; non-final fragments left the field untouched.  If a
> descriptor slot was reused while it still held a stale true from
> a previous SKB (possible when tx_spb_ring_full() underreported
> fullness), bcmasp_tx_reclaim() would see last == true mid-SKB and
> call dev_consume_skb_any() prematurely, freeing the sk_buff while
> its remaining fragments were still in flight.
> 
> Unconditionally clear txcb->last before the conditional set so every
> descriptor slot starts from a known false state regardless of what a
> prior transmission left behind.
> 
> Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller")
> Signed-off-by: Justin Chen <justin.chen@broadcom.com>
> Assisted-by: Claude:claude-sonnet-4-6 vscode
> Signed-off-by: Danesh Petigara <danesh.petigara@broadcom.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[PATCH net 2/2] net: bcmasp: fix tx_spb_ring_full() checking same slot cnt times

[Danesh Petigara]

From: Justin Chen <justin.chen@broadcom.com>

The loop initialised next_index from intf->tx_spb_index on every
iteration, so incr_ring() always produced the same result and only
one slot was ever tested.  Move the initialisation before the loop
so each iteration advances next_index and the function correctly
checks that cnt consecutive descriptor slots are available before
allowing a new transmission.

Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller")
Signed-off-by: Justin Chen <justin.chen@broadcom.com>
Assisted-by: Claude:claude-sonnet-4-6 vscode
Signed-off-by: Danesh Petigara <danesh.petigara@broadcom.com>
---
 drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c
index 2bd035f74fa2..f2176ef3a127 100644
--- a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c
+++ b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c
@@ -148,8 +148,9 @@ static int tx_spb_ring_full(struct bcmasp_intf *intf, int cnt)
 	int next_index, i;
 
 	/* Check if we have enough room for cnt descriptors */
+	next_index = intf->tx_spb_index;
 	for (i = 0; i < cnt; i++) {
-		next_index = incr_ring(intf->tx_spb_index, DESC_RING_COUNT);
+		next_index = incr_ring(next_index, DESC_RING_COUNT);
 		if (next_index == intf->tx_spb_clean_index)
 			return 1;
 	}
-- 
2.54.0

Re: [PATCH net 2/2] net: bcmasp: fix tx_spb_ring_full() checking same slot cnt times

[Florian Fainelli]

On 8/31/26 11:42, Danesh Petigara wrote:
> From: Justin Chen <justin.chen@broadcom.com>
> 
> The loop initialised next_index from intf->tx_spb_index on every
> iteration, so incr_ring() always produced the same result and only
> one slot was ever tested.  Move the initialisation before the loop
> so each iteration advances next_index and the function correctly
> checks that cnt consecutive descriptor slots are available before
> allowing a new transmission.
> 
> Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller")
> Signed-off-by: Justin Chen <justin.chen@broadcom.com>
> Assisted-by: Claude:claude-sonnet-4-6 vscode
> Signed-off-by: Danesh Petigara <danesh.petigara@broadcom.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net 0/2] net: bcmasp: fix TX ring accounting bugs

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 31 Aug 2026 11:42:33 -0700 you wrote:
> Two fixes for TX descriptor ring handling in the bcmasp driver:
> 
>   - tx_spb_ring_full() re-initialized next_index from
>     intf->tx_spb_index on every loop iteration instead of advancing
>     it, so it only ever checked a single descriptor slot regardless
>     of cnt. This let bcmasp_xmit() proceed even when the ring didn't
>     actually have enough free slots for the SKB's fragments.
> 
> [...]

Here is the summary with links:
  - [net,1/2] net: bcmasp: clear txcb->last before writing each descriptor
    https://git.kernel.org/netdev/net/c/18e5e0ec0e92
  - [net,2/2] net: bcmasp: fix tx_spb_ring_full() checking same slot cnt times
    https://git.kernel.org/netdev/net/c/0c5cf62e72d7

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