Re: [PATCH net] net: ti: icssg-prueth: fix use-after-free of CPPI descriptor in RX path

[Simon Horman <horms@kernel.org>]

On Thu, Mar 19, 2026 at 06:12:36PM +0000, David Carlier wrote:
> cppi5_hdesc_get_psdata() returns a pointer into the CPPI descriptor.
> In both emac_rx_packet() and emac_rx_packet_zc(), the descriptor is
> freed via k3_cppi_desc_pool_free() before the psdata pointer is used
> by emac_rx_timestamp(), which dereferences psdata[0] and psdata[1].
> This constitutes a use-after-free on every received packet that goes
> through the timestamp path.
> 
> Defer the descriptor free until after all accesses through the psdata
> pointer are complete. For emac_rx_packet(), this means freeing in each
> early-exit path and after emac_rx_timestamp() in the success path. For
> emac_rx_packet_zc(), move the free to the end of the loop body after
> emac_dispatch_skb_zc() (which calls emac_rx_timestamp()) has returned.
> 
> Fixes: 46eeb90f03e0 ("net: ti: icssg-prueth: Use page_pool API for RX buffer allocation")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  drivers/net/ethernet/ti/icssg/icssg_common.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c

...

> @@ -1057,7 +1057,6 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, u32 *xdp_state)
>  	/* firmware adds 4 CRC bytes, strip them */
>  	pkt_len -= 4;
>  	cppi5_desc_get_tags_ids(&desc_rx->hdr, &port_id, NULL);
> -	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
>  
>  	/* if allocation fails we drop the packet but push the
>  	 * descriptor back to the ring with old page to prevent a stall
> @@ -1066,6 +1065,7 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, u32 *xdp_state)
>  	if (unlikely(!new_page)) {
>  		new_page = page;
>  		ndev->stats.rx_dropped++;
> +		k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
>  		goto requeue;

Hi David,

I am sorry for not noticing this earlier.

It seems to me that with this patch k3_cppi_desc_pool_free()
is always called before the requeue label is reached.

Would it make sense to move the call to k3_cppi_desc_pool_free()
into the requeue label? Or does it need to be called earlier
than that in the non-goto case?

>  	}
>  

...