Re: [PATCH net] net: macb: drop in-flight Tx SKBs on close

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi Théo,

thanks for your patch.

On 24.4.2026 12:01, Théo Lebrun wrote:
> [...]

> 
>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> +		dropped = CIRC_CNT(queue->tx_head, queue->tx_tail,
> +				   bp->tx_ring_size);
> +		queue->stats.tx_dropped += dropped;
> +		bp->dev->stats.tx_dropped += dropped;

AFAIUI CIRC_CNT counts descriptor slots, not packets. A fragmented
skb uses multiple slots so tx_dropped would be overcounted?

> +
> +		for (tail = queue->tx_tail; tail != queue->tx_head; tail++)
> +			macb_tx_unmap(bp, macb_tx_skb(queue, tail), 0);

I might be missing something, but couldn't this crash on the
macb_alloc_consistent() -> out_err path after a previous close
with in-flight frames?

1. First close: the new loop runs and frees skbs, but tx_head and
    tx_tail are not reset. kfree(tx_skb) sets it to NULL.
2. Second open: macb_alloc_consistent() fails early (e.g. the tx
    dma_alloc_coherent on the first line) and jumps to out_err.
3. macb_free_consistent() runs again. CIRC_CNT is non-zero (stale
    from previous session). macb_tx_skb() dereferences queue->tx_skb
    which is NULL.

Or if the failure happens later, the loop would iterate over a
freshly kmalloc'd (uninitialized) tx_skb array and macb_tx_unmap()
would read garbage mapping/skb pointers.

Maybe reset tx_head = tx_tail = 0 after the loop, or guard with
if (queue->tx_skb)?

> +
>  		kfree(queue->tx_skb);
>  		queue->tx_skb = NULL;
>  		queue->tx_ring = NULL;
> 

> [...]

Thanks,
Nicolai