Re: [PATCH ath-next] wifi: ath9k: unmap DMA and free skb for stale TX buffers on queue drain

[Toke Høiland-Jørgensen]

Rosen Penev <rosenp@xxxxxxxxx> writes:

> When draining a TX queue (e.g. on device removal or reset), stale
> ath_buf descriptors were returned directly to the free pool via
> ath_tx_return_buffer without unmapping their DMA buffers or freeing
> their associated skbs. This leaks both DMA API resources and socket
> buffer memory.
>
> Unmap the DMA buffer and free the skb before returning the buffer
> to the pool.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> ---
>  drivers/net/wireless/ath/ath9k/xmit.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 89d8b3178784..89930eaf1fd3 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -1884,8 +1884,17 @@ static void ath_drain_txq_list(struct ath_softc *sc, struct ath_txq *txq,
>  		bf = list_first_entry(list, struct ath_buf, list);
>  
>  		if (bf->bf_state.stale) {
> -			list_del(&bf->list);
> +			struct sk_buff *skb = bf->bf_mpdu;
>  
> +			list_del(&bf->list);
> +			if (bf->bf_buf_addr) {
> +				dma_unmap_single(sc->dev, bf->bf_buf_addr,
> +						 skb ? skb->len : 0,
> +						 DMA_TO_DEVICE);

A zero-length unmap? That seems completely non-sensical. Is there even a
bug here? AFAICT, the only place where a buf is set as stale, the
aggregate subsequently goes through ath_tx_complete_buf(), which does
the unmapping.

Did you test this? Unless you can demonstrate a real bug here, I'm just
going to assume this is all LLM garbage output.

-Toke