[PATCH net] net: airoha: Fix memory leak in airoha_qdma_rx_process()

[PATCH net] net: airoha: Fix memory leak in airoha_qdma_rx_process()

[Lorenzo Bianconi]

If an error occurs on the subsequents buffers belonging to the
non-linear part of the skb (e.g. due to an error in the payload length
reported by the NIC or if we consumed all the available fragments for
the skb), the page_pool fragment will not be linked to the skb so it will
not return to the pool in the airoha_qdma_rx_process() error path. Fix the
memory leak partially reverting commit 'd6d2b0e1538d ("net: airoha: Fix
page recycling in airoha_qdma_rx_process()")' and always running
page_pool_put_full_page routine in the airoha_qdma_rx_process() error
path.

Fixes: d6d2b0e1538d ("net: airoha: Fix page recycling in airoha_qdma_rx_process()")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 95ba99b89428e4cafb91ff7813e43ffeb38e6d9b..91cb63a32d9904e0700bcce45b53624677d75c6c 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -697,9 +697,8 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
 		if (q->skb) {
 			dev_kfree_skb(q->skb);
 			q->skb = NULL;
-		} else {
-			page_pool_put_full_page(q->page_pool, page, true);
 		}
+		page_pool_put_full_page(q->page_pool, page, true);
 	}
 	airoha_qdma_fill_rx_queue(q);
 

---
base-commit: a1822cb524e89b4cd2cf0b82e484a2335496a6d9
change-id: 20260402-airoha_qdma_rx_process-mem-leak-fix-27b53dbaaa4f

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>

Re: [PATCH net] net: airoha: Fix memory leak in airoha_qdma_rx_process()

[Simon Horman]

On Thu, Apr 02, 2026 at 02:57:10PM +0200, Lorenzo Bianconi wrote:
> If an error occurs on the subsequents buffers belonging to the
> non-linear part of the skb (e.g. due to an error in the payload length
> reported by the NIC or if we consumed all the available fragments for
> the skb), the page_pool fragment will not be linked to the skb so it will
> not return to the pool in the airoha_qdma_rx_process() error path. Fix the
> memory leak partially reverting commit 'd6d2b0e1538d ("net: airoha: Fix
> page recycling in airoha_qdma_rx_process()")' and always running
> page_pool_put_full_page routine in the airoha_qdma_rx_process() error
> path.
> 
> Fixes: d6d2b0e1538d ("net: airoha: Fix page recycling in airoha_qdma_rx_process()")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net] net: airoha: Fix memory leak in airoha_qdma_rx_process()

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email says: "more thing to fix.."
---
net: airoha: Fix memory leak in airoha_qdma_rx_process()

This commit fixes a memory leak in the error path of
airoha_qdma_rx_process() by partially reverting a previous commit and
always calling page_pool_put_full_page().

> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 95ba99b89428e..91cb63a32d990 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -697,9 +697,8 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)

This problem wasn't introduced by this patch, but while reviewing this
function I noticed the descriptor field msg1 is read from memory before
the ownership bit is validated:

airoha_qdma_rx_process() {
        while (done < budget) {
                struct airoha_queue_entry *e = &q->entry[q->tail];
                struct airoha_qdma_desc *desc = &q->desc[q->tail];
                u32 hash, reason, msg1 = le32_to_cpu(desc->msg1);
                struct page *page = virt_to_head_page(e->buf);
                u32 desc_ctrl = le32_to_cpu(desc->ctrl);

                if (!(desc_ctrl & QDMA_DESC_DONE_MASK))
                        break;

Could this lead to processing stale descriptor metadata? On weakly-ordered
architectures, the CPU might reorder these reads before the device commits
the new descriptor data. 

Are we missing a dma_rmb() after the ownership check?

>  		if (q->skb) {
>  			dev_kfree_skb(q->skb);
>  			q->skb = NULL;
> -		} else {
> -			page_pool_put_full_page(q->page_pool, page, true);
>  		}

This is also an existing issue, but does jumping to this error path on a
scattered packet fragment leave the receive loop in an inconsistent state?

If an error occurs while processing a middle fragment of a packet, q->skb
is freed and set to NULL here. However, the loop continues to the next
descriptor.

Since q->skb is now NULL, won't the next fragment of the same corrupted
packet be erroneously parsed as the start of a brand new Ethernet frame:

                if (!q->skb) { /* first buffer */
                        q->skb = napi_build_skb(e->buf, q->buf_size);

Could this desynchronization allow arbitrary payload to be injected into
the network stack as a legitimate packet? The driver may need to discard
all subsequent fragments until a descriptor without QDMA_DESC_MORE_MASK is
encountered.

> +		page_pool_put_full_page(q->page_pool, page, true);
>  	}
>  	airoha_qdma_fill_rx_queue(q);
>

Re: [PATCH net] net: airoha: Fix memory leak in airoha_qdma_rx_process()

[patchwork-bot+netdevbpf]

Hello:

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

On Thu, 02 Apr 2026 14:57:10 +0200 you wrote:
> If an error occurs on the subsequents buffers belonging to the
> non-linear part of the skb (e.g. due to an error in the payload length
> reported by the NIC or if we consumed all the available fragments for
> the skb), the page_pool fragment will not be linked to the skb so it will
> not return to the pool in the airoha_qdma_rx_process() error path. Fix the
> memory leak partially reverting commit 'd6d2b0e1538d ("net: airoha: Fix
> page recycling in airoha_qdma_rx_process()")' and always running
> page_pool_put_full_page routine in the airoha_qdma_rx_process() error
> path.
> 
> [...]

Here is the summary with links:
  - [net] net: airoha: Fix memory leak in airoha_qdma_rx_process()
    https://git.kernel.org/netdev/net/c/285fa6b1e03c

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