Re: [PATCH net-next v1 08/11] eth: fbnic: Support XDP redirect

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] eth: fbnic: memory leak in fbnic_pkt_prepare() [1]
--

commit 8a7293bd4896ffd0472ecd918ba40c84ba4499d4
Author: Björn Töpel <bjorn@kernel.org>

eth: fbnic: Support XDP redirect

This commit adds support for the XDP_REDIRECT action in fbnic, including
flushing redirected packets at the end of the RX completion poll and
advertising NETDEV_XDP_ACT_REDIRECT.

> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
> index 0bcee152d47c3..51d573c9c5b83 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
[ ... ]
> @@ -1336,6 +1343,16 @@ static int fbnic_clean_rcq(struct fbnic_napi_vector *nv,
>  				pkt_tail = nv->qt[0].sub1.tail;
>  			} else if (PTR_ERR(skb) == -FBNIC_XDP_CONSUME) {
>  				fbnic_put_pkt_buff(qt, pkt, 1);
> +			} else if (skb == ERR_PTR(-FBNIC_XDP_REDIRECT)) {
> +				redirect = true;
> +			} else if (skb == ERR_PTR(-FBNIC_XDP_REDIRECT_ERR)) {
> +				/* A broadcast may queue frames, then fail. XDP
> +				 * consumed the packet, so it also counts as
> +				 * received.
> +				 */
> +				redirect = true;
> +				dropped++;
> +				fbnic_put_pkt_buff(qt, pkt, 1);

[Severity: High]
This problem wasn't introduced by this patch, but can fbnic_pkt_prepare()
leak the existing packet buffer [1]?

If fbnic_clean_rcq() sees consecutive FBNIC_RCD_TYPE_HDR_AL descriptors
without an intervening FBNIC_RCD_TYPE_META descriptor, it calls
fbnic_pkt_prepare() again on the same pkt:

drivers/net/ethernet/meta/fbnic/fbnic_txrx.c:fbnic_pkt_prepare() {
	...
	/* data_hard_start should always be NULL when this is called */
	WARN_ON_ONCE(pkt->buff.data_hard_start);
	...
	xdp_init_buff(&pkt->buff, frame_sz, &qt->xdp_rxq);
	...
}

Reinitializing pkt->buff without first calling fbnic_put_pkt_buff() drops
the reference to the previously allocated head page and any attached
fragments, allowing a malfunctioning or compromised device to leak page_pool
buffers.

[1] https://sashiko.dev/#/bug/linux-155b4068-9e56-4cd0-bea5-9b3a787a5d5d

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110102039.1002605-1-bjorn@kernel.org?part=8