Re: [PATCH net-next v2 2/3] net: atlantic: free RX pages of consumed but not refilled buffers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The PTP TX ring leaks SKBs and their DMA mappings on shutdown or error unwind paths if the hardware has not completed transmission of all queued packets.
--

--- Patch [2]: [PATCH net-next v2 2/3] net: atlantic: free RX pages of consumed but not refilled buffers ---
commit 7a64d2b15db815d6730701e2ca08b6013f4f4ae8
Author: Yangyu Chen <cyy@cyyself.name>

net: atlantic: free RX pages of consumed but not refilled buffers
    
This commit fixes a memory and DMA mapping leak in the atlantic driver that
occurs when the interface is brought down. It modifies aq_ring_rx_deinit()
to exhaustively walk the entire RX ring and release any remaining pages,
rather than only iterating over the region posted to hardware.

> Walk the whole ring instead and release whatever is still there. Also
> bail out if the buffer ring is already gone: a partial
> aq_ptp_ring_alloc() failure frees the ring but leaves aq_nic set, so
> aq_ptp_ring_deinit() still gets here on the unwind path.

[Severity: Medium]
This is a pre-existing issue, but since the commit message mentions the
unwind path via aq_ptp_ring_deinit(), I noticed how the PTP TX ring is
cleaned up:

drivers/net/ethernet/aquantia/atlantic/aq_ptp.c:aq_ptp_ring_deinit() {
    ...
	aq_ring_tx_clean(&aq_ptp->ptp_tx);
	aq_ring_rx_deinit(&aq_ptp->ptp_rx);
}

Does this code leak SKBs and their DMA mappings on the shutdown or error
unwind paths if the hardware has not completed transmission of all queued
packets?

Since aq_ring_tx_clean() only iterates up to hw_head, it leaves pending
descriptors uncleaned. Should there be an exhaustive aq_ring_tx_deinit()
call instead?

> Fixes: 46f4c29d9de6 ("net: aquantia: optimize rx performance by page reuse strategy")
> Cc: stable@vger.kernel.org # v5.2+
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> index 81685a4dc5a6d..e1193c6719d9d 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_1F173E0FC1606D2AC704DC9C98AF10984607@qq.com?part=2