[PATCH] net: atlantic: fix fragment overflow handling in RX path

[PATCH] net: atlantic: fix fragment overflow handling in RX path

[Jiefeng]

From f78a25e62b4a0155beee0449536ba419feeddb75 Mon Sep 17 00:00:00 2001
From: Jiefeng Zhang <jiefeng.z.zhang@gmail.com>
Date: Mon, 17 Nov 2025 16:17:37 +0800
Subject: [PATCH] net: atlantic: fix fragment overflow handling in RX path

The atlantic driver can receive packets with more than MAX_SKB_FRAGS (17)
fragments when handling large multi-descriptor packets. This causes an
out-of-bounds write in skb_add_rx_frag_netmem() leading to kernel panic.

The issue occurs because the driver doesn't check the total number of
fragments before calling skb_add_rx_frag(). When a packet requires more
than MAX_SKB_FRAGS fragments, the fragment index exceeds the array bounds.

Add a check in __aq_ring_rx_clean() to ensure the total number of fragments
(including the initial header fragment and subsequent descriptor fragments)
does not exceed MAX_SKB_FRAGS. If it does, drop the packet gracefully
and increment the error counter.

Signed-off-by: Jiefeng Zhang <jiefeng.z.zhang@gmail.com>
---
 .../net/ethernet/aquantia/atlantic/aq_ring.c  | 26 ++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index f21de0c21e52..51e0c6cc71d7 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -538,6 +538,7 @@ static int __aq_ring_rx_clean(struct aq_ring_s
*self, struct napi_struct *napi,
  bool is_ptp_ring = aq_ptp_ring(self->aq_nic, self);
  struct aq_ring_buff_s *buff_ = NULL;
  struct sk_buff *skb = NULL;
+ unsigned int frag_cnt = 0U;
  unsigned int next_ = 0U;
  unsigned int i = 0U;
  u16 hdr_len;
@@ -546,7 +547,6 @@ static int __aq_ring_rx_clean(struct aq_ring_s
*self, struct napi_struct *napi,
  continue;

  if (!buff->is_eop) {
- unsigned int frag_cnt = 0U;
  buff_ = buff;
  do {
  bool is_rsc_completed = true;
@@ -628,6 +628,30 @@ static int __aq_ring_rx_clean(struct aq_ring_s
*self, struct napi_struct *napi,
    aq_buf_vaddr(&buff->rxdata),
    AQ_CFG_RX_HDR_SIZE);

+ /* Check if total fragments exceed MAX_SKB_FRAGS limit.
+ * The total fragment count consists of:
+ * - One fragment from the first buffer if (buff->len > hdr_len)
+ * - frag_cnt fragments from subsequent descriptors
+ * If the total exceeds MAX_SKB_FRAGS (17), we must drop the
+ * packet to prevent an out-of-bounds write in skb_add_rx_frag().
+ */
+ if (unlikely(((buff->len - hdr_len) > 0 ? 1 : 0) + frag_cnt >
MAX_SKB_FRAGS)) {
+ /* Drop packet: fragment count exceeds kernel limit */
+ if (!buff->is_eop) {
+ buff_ = buff;
+ do {
+ next_ = buff_->next;
+ buff_ = &self->buff_ring[next_];
+ buff_->is_cleaned = 1;
+ } while (!buff_->is_eop);
+ }
+ u64_stats_update_begin(&self->stats.rx.syncp);
+ ++self->stats.rx.errors;
+ u64_stats_update_end(&self->stats.rx.syncp);
+ dev_kfree_skb_any(skb);
+ continue;
+ }
+
  memcpy(__skb_put(skb, hdr_len), aq_buf_vaddr(&buff->rxdata),
         ALIGN(hdr_len, sizeof(long)));

--
2.39.5

Re: [PATCH] net: atlantic: fix fragment overflow handling in RX path

[Jakub Kicinski]

On Mon, 17 Nov 2025 19:38:54 +0800 Jiefeng wrote:
> From f78a25e62b4a0155beee0449536ba419feeddb75 Mon Sep 17 00:00:00 2001
> From: Jiefeng Zhang <jiefeng.z.zhang@gmail.com>
> Date: Mon, 17 Nov 2025 16:17:37 +0800
> Subject: [PATCH] net: atlantic: fix fragment overflow handling in RX path
> 
> The atlantic driver can receive packets with more than MAX_SKB_FRAGS (17)
> fragments when handling large multi-descriptor packets. This causes an
> out-of-bounds write in skb_add_rx_frag_netmem() leading to kernel panic.
> 
> The issue occurs because the driver doesn't check the total number of
> fragments before calling skb_add_rx_frag(). When a packet requires more
> than MAX_SKB_FRAGS fragments, the fragment index exceeds the array bounds.
> 
> Add a check in __aq_ring_rx_clean() to ensure the total number of fragments
> (including the initial header fragment and subsequent descriptor fragments)
> does not exceed MAX_SKB_FRAGS. If it does, drop the packet gracefully
> and increment the error counter.

This submissions is not formatted correctly. Use git or b4 to send your
code. Please also make sure you read:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html

As for the patch -- what's the frag size the driver uses? If it's 
larger than max_mtu / 16 the overflow is impossible.

Re: [PATCH] net: atlantic: fix fragment overflow handling in RX path

[Jiefeng]

I have used git send-email to send my code.

As for the patch --The aquantia/atlantic driver supports a maximum of
AQ_CFG_SKB_FRAGS_MAX (32U) fragments, while the kernel limits the
maximum number of fragments to MAX_SKB_FRAGS (17).

Jakub Kicinski <kuba@kernel.org> 于2025年11月18日周二 08:42写道：
>
> On Mon, 17 Nov 2025 19:38:54 +0800 Jiefeng wrote:
> > From f78a25e62b4a0155beee0449536ba419feeddb75 Mon Sep 17 00:00:00 2001
> > From: Jiefeng Zhang <jiefeng.z.zhang@gmail.com>
> > Date: Mon, 17 Nov 2025 16:17:37 +0800
> > Subject: [PATCH] net: atlantic: fix fragment overflow handling in RX path
> >
> > The atlantic driver can receive packets with more than MAX_SKB_FRAGS (17)
> > fragments when handling large multi-descriptor packets. This causes an
> > out-of-bounds write in skb_add_rx_frag_netmem() leading to kernel panic.
> >
> > The issue occurs because the driver doesn't check the total number of
> > fragments before calling skb_add_rx_frag(). When a packet requires more
> > than MAX_SKB_FRAGS fragments, the fragment index exceeds the array bounds.
> >
> > Add a check in __aq_ring_rx_clean() to ensure the total number of fragments
> > (including the initial header fragment and subsequent descriptor fragments)
> > does not exceed MAX_SKB_FRAGS. If it does, drop the packet gracefully
> > and increment the error counter.
>
> This submissions is not formatted correctly. Use git or b4 to send your
> code. Please also make sure you read:
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
>
> As for the patch -- what's the frag size the driver uses? If it's
> larger than max_mtu / 16 the overflow is impossible.

[PATCH] net: atlantic: fix fragment overflow handling in RX path

[Jiefeng]

From: Jiefeng Zhang <jiefeng.z.zhang@gmail.com>
Date: Mon, 17 Nov 2025 16:17:37 +0800
Subject: [PATCH]  net: atlantic: fix fragment overflow handling in RX path

The atlantic driver can receive packets with more than
MAX_SKB_FRAGS (17) fragments when handling large multi-descriptor packets.
This causes an out-of-bounds write in skb_add_rx_frag_netmem() leading to
kernel panic.

The issue occurs because the driver doesn't check the total number of
fragments before calling skb_add_rx_frag(). When a packet requires more
than MAX_SKB_FRAGS fragments, the fragment index exceeds the array bounds.

Add a check in __aq_ring_rx_clean() to ensure the total number of fragments
(including the initial header fragment and subsequent descriptor fragments)
does not exceed MAX_SKB_FRAGS. If it does, drop the packet gracefully
and increment the error counter.

Signed-off-by: Jiefeng Zhang <jiefeng.z.zhang@gmail.com>
---
 .../net/ethernet/aquantia/atlantic/aq_ring.c  | 26 ++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index f21de0c21e52..51e0c6cc71d7 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -538,6 +538,7 @@ static int __aq_ring_rx_clean(struct aq_ring_s
*self, struct napi_struct *napi,
  bool is_ptp_ring = aq_ptp_ring(self->aq_nic, self);
  struct aq_ring_buff_s *buff_ = NULL;
  struct sk_buff *skb = NULL;
+ unsigned int frag_cnt = 0U;
  unsigned int next_ = 0U;
  unsigned int i = 0U;
  u16 hdr_len;
@@ -546,7 +547,6 @@ static int __aq_ring_rx_clean(struct aq_ring_s
*self, struct napi_struct *napi,
  continue;

  if (!buff->is_eop) {
- unsigned int frag_cnt = 0U;
  buff_ = buff;
  do {
  bool is_rsc_completed = true;
@@ -628,6 +628,30 @@ static int __aq_ring_rx_clean(struct aq_ring_s
*self, struct napi_struct *napi,
    aq_buf_vaddr(&buff->rxdata),
    AQ_CFG_RX_HDR_SIZE);

+ /* Check if total fragments exceed MAX_SKB_FRAGS limit.
+ * The total fragment count consists of:
+ * - One fragment from the first buffer if (buff->len > hdr_len)
+ * - frag_cnt fragments from subsequent descriptors
+ * If the total exceeds MAX_SKB_FRAGS (17), we must drop the
+ * packet to prevent an out-of-bounds write in skb_add_rx_frag().
+ */
+ if (unlikely(((buff->len - hdr_len) > 0 ? 1 : 0) + frag_cnt >
MAX_SKB_FRAGS)) {
+ /* Drop packet: fragment count exceeds kernel limit */
+ if (!buff->is_eop) {
+ buff_ = buff;
+ do {
+ next_ = buff_->next;
+ buff_ = &self->buff_ring[next_];
+ buff_->is_cleaned = 1;
+ } while (!buff_->is_eop);
+ }
+ u64_stats_update_begin(&self->stats.rx.syncp);
+ ++self->stats.rx.errors;
+ u64_stats_update_end(&self->stats.rx.syncp);
+ dev_kfree_skb_any(skb);
+ continue;
+ }
+
  memcpy(__skb_put(skb, hdr_len), aq_buf_vaddr(&buff->rxdata),
         ALIGN(hdr_len, sizeof(long)));

-- 
2.39.5