Message-ID: <sashiko-outbox-206188@kernel.org>
From: sashiko-bot@kernel.org
Subject: Re: [PATCH bpf-next 11/15] ice: set skb RX metadata from the skb
 core extension area of xdp_buff
Reply-To: sashiko-reviews@lists.linux.dev
To: "Tianyi Gao" <tianyi@cloudflare.com>, bpf@vger.kernel.org
Cc: intel-wired-lan@lists.osuosl.org
In-Reply-To: <ae488c057454cd57fb01ac01aefc3e97d6271ada.1794281983.git.tianyi@cloudflare.com>
References: <cover.1794281983.git.tianyi@cloudflare.com>
 <ae488c057454cd57fb01ac01aefc3e97d6271ada.1794281983.git.tianyi@cloudflare.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Tue, 10 Nov 2026 04:23:22 +0000
Precedence: list

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:
- [Critical] ice: invalid page dereference and page leak in
  ice_clean_rx_irq() [1]
--

commit ba151a63223661db4511ce1634b36c26afe91c8f
Author: Tianyi Gao <tianyi@cloudflare.com>

ice: set skb RX metadata from the skb core extension area of xdp_buff

This commit populates skb RX metadata from the xdp_buff skb core extension
area on XDP_PASS in the ice driver and exports xdp_skb_ext_to_skb().

> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethe=
rnet/intel/ice/ice_txrx.c
> index a2c7c4962882d..06a1a5fdc43b7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
[ ... ]
> @@ -1041,6 +1042,7 @@ static int ice_clean_rx_irq(struct ice_rx_ring *rx_=
ring, int budget)
>  		continue;
> =20
>  construct_skb:
> +		ext =3D xdp_buff_skb_ext(&xdp->base);
>  		skb =3D xdp_build_skb_from_buff(&xdp->base);
>  		xdp->data =3D NULL;

[Severity: Critical]
This problem wasn't introduced by this patch, but clearing xdp->data before
checking whether xdp_build_skb_from_buff() succeeded causes an invalid page
dereference and leaks the head buffer [1] when skb allocation fails:

drivers/net/ethernet/intel/ice/ice_txrx.c:ice_clean_rx_irq() {
	...
construct_skb:
		ext =3D xdp_buff_skb_ext(&xdp->base);
		skb =3D xdp_build_skb_from_buff(&xdp->base);
		xdp->data =3D NULL;

		/* exit if we failed to retrieve a buffer */
		if (!skb) {
			libeth_xdp_return_buff_slow(xdp);
	...
}

When napi_build_skb() fails inside xdp_build_skb_from_buff(), it returns
NULL without freeing the page_pool buffers attached to xdp. Because
xdp->data is already set to NULL, libeth_xdp_return_buff_slow() passes NULL
to libeth_xdp_return_va(), which translates NULL via virt_to_netmem() and
dereferences an invalid page pointer while leaking the head page.

Should xdp->data =3D NULL be moved after the !skb check (since
__libeth_xdp_return_buff() already clears xdp->data on the failure path)?

[1] https://sashiko.dev/#/bug/linux-d6eb94b5-df62-4b78-9f73-358fdd002a0c

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/cover.1794281983.gi=
t.tianyi@cloudflare.com?part=3D11