Re: [PATCH net-next v2 2/2] net: mana: Use kvmalloc for large RX queue and buffer allocations

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] The error path in `mana_create_rxq()` accesses and attempts to double-free a CQ object embedded within an `rxq` structure that has already been freed, leading to a Use-After-Free and Double Free.
--

commit 2b2a9ca2dc8ac87d8832a2a8e0d3a6c8f2d70ca9
Author: Aditya Garg <gargaditya@linux.microsoft.com>

net: mana: Use kvmalloc for large RX queue and buffer allocations

This commit switches the RX path allocations for pre-allocated buffers and
queues from kmalloc to kvmalloc variants to handle fragmented contiguous
memory, and updates the free paths to use kvfree.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 8adf72b96145..e1d8ac3417e8 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
[ ... ]
> @@ -2564,7 +2564,7 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
>  	if (rxq->gdma_rq)
>  		mana_gd_destroy_queue(gc, rxq->gdma_rq);
>  
> -	kfree(rxq);
> +	kvfree(rxq);
>  }

Does this change to kvfree() expose an existing use-after-free in the error
path of mana_create_rxq()?

In mana_create_rxq(), cq is initialized as a pointer into the rxq structure:

	cq = &rxq->rx_cq;

If an error occurs later in the function, the cleanup path calls
mana_destroy_rxq(), which frees rxq, and then it accesses cq again:

out:
	if (!err)
		return rxq;

	netdev_err(ndev, "Failed to create RXQ: err = %d\n", err);

	mana_destroy_rxq(apc, rxq, false);

	if (cq)
		mana_deinit_cq(apc, cq);

Because mana_destroy_rxq() now uses kvfree(rxq), if the allocation fell back
to vmalloc, the memory will be unmapped immediately. Can accessing cq inside
mana_deinit_cq() trigger a page fault here?

Also, since mana_destroy_rxq() already calls mana_deinit_cq(apc, &rxq->rx_cq)
before freeing rxq, does calling it a second time in the out path result in a
double-free?

> @@ -2704,7 +2704,7 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
>  
>  	gc = gd->gdma_context;
>  
> -	rxq = kzalloc_flex(*rxq, rx_oobs, apc->rx_queue_size);
> +	rxq = kvzalloc_flex(*rxq, rx_oobs, apc->rx_queue_size);
>  	if (!rxq)
>  		return NULL;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260427132807.1642290-1-gargaditya@linux.microsoft.com?part=2