[PATCH net v4 0/3] octeon_ep, octeon_ep_vf: fix RX skb frags overflow and page leak

[PATCH net v4 0/3] octeon_ep, octeon_ep_vf: fix RX skb frags overflow and page leak

[Maoyi Xie]

The octeon_ep and octeon_ep_vf RX paths add one skb fragment per buffer
with no bound against MAX_SKB_FRAGS. buff_info->len comes from the device
response header. A long packet needs about 18 fragments. That is one past
the default MAX_SKB_FRAGS of 17, so skb_add_rx_frag() writes past
shinfo->frags[]. Patch 1 bounds octeon_ep. Patch 3 bounds octeon_ep_vf.

Patch 2 is Guangshuo Li's fix for an octeon_ep_vf RX page leak on the
napi_build_skb() failure path. It touches the same drop code as patch 3.
I carry it here so the series applies without conflict, per Maciej. Patch 3
moves that drain loop into a helper. The helper carries the page frees from
patch 2, so the overflow drop path frees its pages too.

octeon_ep has the same leak on its drop path. A separate patch will fix it
once this series lands.

v4:
 - add Guangshuo Li's octeon_ep_vf page leak fix as patch 2, per Maciej.
 - octeon_ep_vf: the fragment count fix now sits on top of patch 2. The drop
   helper octep_vf_oq_drop_rx() frees the head and fragment pages.
 - octeon_ep: no change, keeps Maciej's Reviewed-by.

v1: https://lore.kernel.org/r/20260701112825.1653044-1-maoyixie.tju@gmail.com
v2: https://lore.kernel.org/r/20260702180518.2013324-1-maoyixie.tju@gmail.com
v3: https://lore.kernel.org/r/20260704061511.2350737-1-maoyixie.tju@gmail.com

Guangshuo Li (1):
  octeon_ep_vf: Fix RX page leak on napi_build_skb() failure

Maoyi Xie (2):
  octeon_ep: fix skb frags overflow in the RX path
  octeon_ep_vf: fix skb frags overflow in the RX path

 .../net/ethernet/marvell/octeon_ep/octep_rx.c |  9 ++++
 .../marvell/octeon_ep_vf/octep_vf_rx.c        | 49 +++++++++++++------
 2 files changed, 42 insertions(+), 16 deletions(-)

-- 
2.34.1

[PATCH net v4 1/3] octeon_ep: fix skb frags overflow in the RX path

[Maoyi Xie]

__octep_oq_process_rx() builds an skb for a multi-buffer packet by adding
one fragment per buffer_size chunk:

	data_len = buff_info->len - oq->max_single_buffer_size;
	while (data_len) {
		...
		skb_add_rx_frag(skb, shinfo->nr_frags, buff_info->page, 0,
				buff_info->len, buff_info->len);
		...
	}

buff_info->len comes from the device response header
(be64_to_cpu(resp_hw->length)). Nothing bounds the fragment count against
MAX_SKB_FRAGS. data_len can be close to 65535. buffer_size defaults to
about 3776 on 4K pages, so a full packet yields about 18 fragments. That
is one more than the default MAX_SKB_FRAGS of 17, so skb_add_rx_frag()
writes past shinfo->frags[].

The fragment count is now checked before build_skb(). A packet that needs
more fragments than the skb can hold is dropped. octep_oq_drop_rx()
consumes its descriptors like the build_skb failure path. The same class
was fixed in other RX paths, including commit 5ffcb7b890f6 ("net: atlantic:
fix fragment overflow handling in RX path") and commit f0813bcd2d9d ("net:
wwan: t7xx: fix potential skb->frags overflow in RX path").

Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
Co-developed-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
Signed-off-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 drivers/net/ethernet/marvell/octeon_ep/octep_rx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
index e6ebc7e44a..bdbed58c7b 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
@@ -453,6 +453,15 @@ static int __octep_oq_process_rx(struct octep_device *oct,
 
 		octep_oq_next_pkt(oq, buff_info, &read_idx, &desc_used);
 
+		if (buff_info->len > oq->max_single_buffer_size) {
+			u16 data_len = buff_info->len - oq->max_single_buffer_size;
+
+			if (DIV_ROUND_UP(data_len, oq->buffer_size) > MAX_SKB_FRAGS) {
+				octep_oq_drop_rx(oq, buff_info, &read_idx, &desc_used);
+				continue;
+			}
+		}
+
 		skb = build_skb((void *)resp_hw, PAGE_SIZE);
 		if (!skb) {
 			octep_oq_drop_rx(oq, buff_info,
-- 
2.34.1

Re: [PATCH net v4 1/3] octeon_ep: fix skb frags overflow in the RX path

[Simon Horman]

On Mon, Jul 06, 2026 at 11:02:06PM +0800, Maoyi Xie wrote:
> __octep_oq_process_rx() builds an skb for a multi-buffer packet by adding
> one fragment per buffer_size chunk:
> 
> 	data_len = buff_info->len - oq->max_single_buffer_size;
> 	while (data_len) {
> 		...
> 		skb_add_rx_frag(skb, shinfo->nr_frags, buff_info->page, 0,
> 				buff_info->len, buff_info->len);
> 		...
> 	}
> 
> buff_info->len comes from the device response header
> (be64_to_cpu(resp_hw->length)). Nothing bounds the fragment count against
> MAX_SKB_FRAGS. data_len can be close to 65535. buffer_size defaults to
> about 3776 on 4K pages, so a full packet yields about 18 fragments. That
> is one more than the default MAX_SKB_FRAGS of 17, so skb_add_rx_frag()
> writes past shinfo->frags[].
> 
> The fragment count is now checked before build_skb(). A packet that needs
> more fragments than the skb can hold is dropped. octep_oq_drop_rx()
> consumes its descriptors like the build_skb failure path. The same class
> was fixed in other RX paths, including commit 5ffcb7b890f6 ("net: atlantic:
> fix fragment overflow handling in RX path") and commit f0813bcd2d9d ("net:
> wwan: t7xx: fix potential skb->frags overflow in RX path").
> 
> Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
> Co-developed-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
> Signed-off-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>  drivers/net/ethernet/marvell/octeon_ep/octep_rx.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
> index e6ebc7e44a..bdbed58c7b 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
> @@ -453,6 +453,15 @@ static int __octep_oq_process_rx(struct octep_device *oct,
>  
>  		octep_oq_next_pkt(oq, buff_info, &read_idx, &desc_used);
>  
> +		if (buff_info->len > oq->max_single_buffer_size) {
> +			u16 data_len = buff_info->len - oq->max_single_buffer_size;
> +
> +			if (DIV_ROUND_UP(data_len, oq->buffer_size) > MAX_SKB_FRAGS) {
> +				octep_oq_drop_rx(oq, buff_info, &read_idx, &desc_used);
> +				continue;
> +			}
> +		}

The AI-generated review of this patch on sashiko.dev flags that
this cannot occur for 4K pages. I believe this can be
addressed by changing the type of data_len to u32.
I also believe that a similar problem exists in patch 3/3.

   "Does truncating the data_len calculation to u16 introduce a regression
    by bypassing the MAX_SKB_FRAGS check?

    If buff_info->len is unexpectedly large, the u16 truncation of data_len
    could wrap around to a small value. This would bypass the check,
    leaving unconsumed fragments in the ring that might be incorrectly
    parsed as new packet headers."

> +
>  		skb = build_skb((void *)resp_hw, PAGE_SIZE);
>  		if (!skb) {
>  			octep_oq_drop_rx(oq, buff_info,
> -- 
> 2.34.1
> 

[PATCH net v4 2/3] octeon_ep_vf: Fix RX page leak on napi_build_skb() failure

[Maoyi Xie]

From: Guangshuo Li <lgs201920130244@gmail.com>

__octep_vf_oq_process_rx() clears buff_info->page before building an skb
from the RX page. On the success path the page is consumed by the skb,
either as the skb head or as an RX fragment.

If napi_build_skb() fails, however, the page is not consumed by an skb.
The error path advances the descriptor and leaves the ring slot cleared,
so the page is no longer tracked and is leaked. In the multi-fragment
case, the remaining fragment pages are also unmapped and removed from
their ring slots without being released.

Release the head page when napi_build_skb() fails, and release each
remaining fragment page before clearing its ring slot.

Fixes: dd66b4285470 ("octeon_ep_vf: add NULL check for napi_build_skb()")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
index d982474082..302559b16b 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
@@ -418,6 +418,7 @@ static int __octep_vf_oq_process_rx(struct octep_vf_device *oct,
 			skb = napi_build_skb((void *)resp_hw, PAGE_SIZE);
 			if (!skb) {
 				oq->stats->alloc_failures++;
+				put_page(virt_to_page(resp_hw));
 				desc_used++;
 				read_idx = octep_vf_oq_next_idx(oq, read_idx);
 				continue;
@@ -434,6 +435,7 @@ static int __octep_vf_oq_process_rx(struct octep_vf_device *oct,
 			skb = napi_build_skb((void *)resp_hw, PAGE_SIZE);
 			if (!skb) {
 				oq->stats->alloc_failures++;
+				put_page(virt_to_page(resp_hw));
 				desc_used++;
 				read_idx = octep_vf_oq_next_idx(oq, read_idx);
 				data_len = buff_info->len - oq->max_single_buffer_size;
@@ -442,6 +444,7 @@ static int __octep_vf_oq_process_rx(struct octep_vf_device *oct,
 						       PAGE_SIZE, DMA_FROM_DEVICE);
 					buff_info = (struct octep_vf_rx_buffer *)
 						    &oq->buff_info[read_idx];
+					put_page(buff_info->page);
 					buff_info->page = NULL;
 					if (data_len < oq->buffer_size)
 						data_len = 0;
-- 
2.34.1

Re: [PATCH net v4 2/3] octeon_ep_vf: Fix RX page leak on napi_build_skb() failure

[Maciej Fijalkowski]

On Mon, Jul 06, 2026 at 11:02:07PM +0800, Maoyi Xie wrote:
> From: Guangshuo Li <lgs201920130244@gmail.com>
> 
> __octep_vf_oq_process_rx() clears buff_info->page before building an skb
> from the RX page. On the success path the page is consumed by the skb,
> either as the skb head or as an RX fragment.
> 
> If napi_build_skb() fails, however, the page is not consumed by an skb.
> The error path advances the descriptor and leaves the ring slot cleared,
> so the page is no longer tracked and is leaked. In the multi-fragment
> case, the remaining fragment pages are also unmapped and removed from
> their ring slots without being released.
> 
> Release the head page when napi_build_skb() fails, and release each
> remaining fragment page before clearing its ring slot.
> 
> Fixes: dd66b4285470 ("octeon_ep_vf: add NULL check for napi_build_skb()")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

> ---
>  drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
> index d982474082..302559b16b 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
> @@ -418,6 +418,7 @@ static int __octep_vf_oq_process_rx(struct octep_vf_device *oct,
>  			skb = napi_build_skb((void *)resp_hw, PAGE_SIZE);
>  			if (!skb) {
>  				oq->stats->alloc_failures++;
> +				put_page(virt_to_page(resp_hw));
>  				desc_used++;
>  				read_idx = octep_vf_oq_next_idx(oq, read_idx);
>  				continue;
> @@ -434,6 +435,7 @@ static int __octep_vf_oq_process_rx(struct octep_vf_device *oct,
>  			skb = napi_build_skb((void *)resp_hw, PAGE_SIZE);
>  			if (!skb) {
>  				oq->stats->alloc_failures++;
> +				put_page(virt_to_page(resp_hw));
>  				desc_used++;
>  				read_idx = octep_vf_oq_next_idx(oq, read_idx);
>  				data_len = buff_info->len - oq->max_single_buffer_size;
> @@ -442,6 +444,7 @@ static int __octep_vf_oq_process_rx(struct octep_vf_device *oct,
>  						       PAGE_SIZE, DMA_FROM_DEVICE);
>  					buff_info = (struct octep_vf_rx_buffer *)
>  						    &oq->buff_info[read_idx];
> +					put_page(buff_info->page);
>  					buff_info->page = NULL;
>  					if (data_len < oq->buffer_size)
>  						data_len = 0;
> -- 
> 2.34.1
> 

[PATCH net v4 3/3] octeon_ep_vf: fix skb frags overflow in the RX path

[Maoyi Xie]

__octep_vf_oq_process_rx() has the same unbounded fragment loop as the PF
driver. buff_info->len comes from the device response header. The loop adds
one fragment per buffer_size chunk with no check against MAX_SKB_FRAGS. A
long packet yields about 18 fragments. That is one past the default
MAX_SKB_FRAGS of 17. skb_add_rx_frag() then writes past shinfo->frags[].

The fragment count is now checked before napi_build_skb(). A packet that
needs more fragments than the skb can hold is dropped.

octep_vf_oq_drop_rx() drains those descriptors. It also frees the head page
and every fragment page. The previous patch added those frees to the inline
drop path. The napi_build_skb() failure path now uses the same helper.

Fixes: 1cd3b407977c ("octeon_ep_vf: add Tx/Rx processing and interrupt support")
Co-developed-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
Signed-off-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
 .../marvell/octeon_ep_vf/octep_vf_rx.c        | 50 ++++++++++++-------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
index 302559b16b..e7733b41a1 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
@@ -357,6 +357,31 @@ static inline u32 octep_vf_oq_next_idx(struct octep_vf_oq *oq, u32 idx)
 	return (idx + 1 == oq->max_count) ? 0 : idx + 1;
 }
 
+static void octep_vf_oq_drop_rx(struct octep_vf_oq *oq,
+				struct octep_vf_rx_buffer *buff_info,
+				void *resp_hw, u32 *read_idx, u32 *desc_used)
+{
+	u16 data_len = buff_info->len - oq->max_single_buffer_size;
+
+	put_page(virt_to_page(resp_hw));
+	(*desc_used)++;
+	*read_idx = octep_vf_oq_next_idx(oq, *read_idx);
+	while (data_len) {
+		dma_unmap_page(oq->dev, oq->desc_ring[*read_idx].buffer_ptr,
+			       PAGE_SIZE, DMA_FROM_DEVICE);
+		buff_info = (struct octep_vf_rx_buffer *)
+			    &oq->buff_info[*read_idx];
+		put_page(buff_info->page);
+		buff_info->page = NULL;
+		if (data_len < oq->buffer_size)
+			data_len = 0;
+		else
+			data_len -= oq->buffer_size;
+		(*desc_used)++;
+		*read_idx = octep_vf_oq_next_idx(oq, *read_idx);
+	}
+}
+
 /**
  * __octep_vf_oq_process_rx() - Process hardware Rx queue and push to stack.
  *
@@ -432,27 +457,16 @@ static int __octep_vf_oq_process_rx(struct octep_vf_device *oct,
 			struct skb_shared_info *shinfo;
 			u16 data_len;
 
+			data_len = buff_info->len - oq->max_single_buffer_size;
+			if (DIV_ROUND_UP(data_len, oq->buffer_size) > MAX_SKB_FRAGS) {
+				octep_vf_oq_drop_rx(oq, buff_info, resp_hw, &read_idx, &desc_used);
+				continue;
+			}
+
 			skb = napi_build_skb((void *)resp_hw, PAGE_SIZE);
 			if (!skb) {
 				oq->stats->alloc_failures++;
-				put_page(virt_to_page(resp_hw));
-				desc_used++;
-				read_idx = octep_vf_oq_next_idx(oq, read_idx);
-				data_len = buff_info->len - oq->max_single_buffer_size;
-				while (data_len) {
-					dma_unmap_page(oq->dev, oq->desc_ring[read_idx].buffer_ptr,
-						       PAGE_SIZE, DMA_FROM_DEVICE);
-					buff_info = (struct octep_vf_rx_buffer *)
-						    &oq->buff_info[read_idx];
-					put_page(buff_info->page);
-					buff_info->page = NULL;
-					if (data_len < oq->buffer_size)
-						data_len = 0;
-					else
-						data_len -= oq->buffer_size;
-					desc_used++;
-					read_idx = octep_vf_oq_next_idx(oq, read_idx);
-				}
+				octep_vf_oq_drop_rx(oq, buff_info, resp_hw, &read_idx, &desc_used);
 				continue;
 			}
 			rx_bytes += buff_info->len;
-- 
2.34.1