* [PATCH net] qede: Fix NULL pointer dereference in TPA fragment processing
@ 2026-07-09 4:47 Vaibhav Nagare
2026-07-16 8:47 ` Simon Horman
0 siblings, 1 reply; 6+ messages in thread
From: Vaibhav Nagare @ 2026-07-09 4:47 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, andrew+netdev
Cc: matvey.kovalev, Pavel.Zhigulin, aelior, manishc, netdev,
linux-kernel, stable, Vaibhav Nagare
Under memory pressure, the qede driver encounters NULL pointer
dereferences when processing TPA continuation fragments because:
1. qede_fill_frag_skb() does not validate the page pointer before use
2. qede_tpa_end() checks error state AFTER calling qede_fill_frag_skb()
The crash occurs when:
1. System experiences memory pressure (GFP_ATOMIC allocations fail)
2. qede_alloc_rx_buffer() returns -ENOMEM, leaving sw_rx_data->data NULL
3. qede_tpa_start() sets QEDE_AGG_STATE_ERROR on SKB allocation failure
4. Hardware delivers TPA_CONT and TPA_END events for this aggregation
5. qede_tpa_end() calls qede_fill_frag_skb() before checking error state
6. qede_fill_frag_skb() accesses NULL pointer in skb_fill_page_desc()
7. Kernel panics with NULL pointer dereference
Example crash from production system:
BUG: unable to handle kernel NULL pointer dereference at 0x8
RIP: qede_fill_frag_skb+0x96/0x430 [qede]
Call Trace:
qede_rx_int+0xb06/0x1de0
qede_poll+0x2f4/0x6c0
__napi_poll+0x2d/0x130
Observed on HPE Synergy 480 Gen11 running RHEL 8.10
(4.18.0-553.134.1.el8_10.x86_64), but the vulnerable code path
exists in mainline.
Fix by:
1. Adding NULL page validation in qede_fill_frag_skb() before dereferencing
2. Checking error state EARLY in qede_tpa_end() before processing fragments
3. Checking error state in qede_tpa_cont() to skip fragment processing
This allows the system to survive memory pressure by dropping packets
instead of crashing.
Fixes: 55482edc25f0 ("qede: Add slowpath/fastpath support and enable hardware GRO")
Cc: stable@vger.kernel.org
Signed-off-by: Vaibhav Nagare <vnagare@redhat.com>
---
drivers/net/ethernet/qlogic/qede/qede_fp.c | 25 ++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 33e18bb69774..95b5cfcc43c2 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -670,13 +670,22 @@ static int qede_fill_frag_skb(struct qede_dev *edev,
NUM_RX_BDS_MAX];
struct qede_agg_info *tpa_info = &rxq->tpa_info[tpa_agg_index];
struct sk_buff *skb = tpa_info->skb;
+ struct page *page = current_bd->data;
if (unlikely(tpa_info->state != QEDE_AGG_STATE_START))
goto out;
+ /* Avoid NULL pointer dereference when under severe memory pressure */
+ if (unlikely(!page)) {
+ DP_NOTICE(edev,
+ "Failed to allocate RX buffer for TPA agg %u\n",
+ tpa_agg_index);
+ goto out;
+ }
+
/* Add one frag and update the appropriate fields in the skb */
skb_fill_page_desc(skb, tpa_info->frag_id++,
- current_bd->data,
+ page,
current_bd->page_offset + rxq->rx_headroom,
len_on_bd);
@@ -684,7 +693,7 @@ static int qede_fill_frag_skb(struct qede_dev *edev,
/* Incr page ref count to reuse on allocation failure
* so that it doesn't get freed while freeing SKB.
*/
- page_ref_inc(current_bd->data);
+ page_ref_inc(page);
goto out;
}
@@ -959,8 +968,16 @@ static inline void qede_tpa_cont(struct qede_dev *edev,
struct qede_rx_queue *rxq,
struct eth_fast_path_rx_tpa_cont_cqe *cqe)
{
+ struct qede_agg_info *tpa_info = &rxq->tpa_info[cqe->tpa_agg_index];
int i;
+ /* Don't process fragments if TPA start failed */
+ if (unlikely(tpa_info->state != QEDE_AGG_STATE_START)) {
+ for (i = 0; i < ARRAY_SIZE(cqe->len_list) && cqe->len_list[i]; i++)
+ qede_recycle_rx_bd_ring(rxq, 1);
+ return;
+ }
+
for (i = 0; i < ARRAY_SIZE(cqe->len_list) && cqe->len_list[i]; i++)
qede_fill_frag_skb(edev, rxq, cqe->tpa_agg_index,
le16_to_cpu(cqe->len_list[i]));
@@ -982,6 +999,10 @@ static int qede_tpa_end(struct qede_dev *edev,
tpa_info = &rxq->tpa_info[cqe->tpa_agg_index];
skb = tpa_info->skb;
+ /* Drop the packet if TPA start failed */
+ if (unlikely(tpa_info->state != QEDE_AGG_STATE_START || !skb))
+ goto err;
+
if (tpa_info->buffer.page_offset == PAGE_SIZE)
dma_unmap_page(rxq->dev, tpa_info->buffer.mapping,
PAGE_SIZE, rxq->data_direction);
--
2.54.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] qede: Fix NULL pointer dereference in TPA fragment processing
2026-07-09 4:47 [PATCH net] qede: Fix NULL pointer dereference in TPA fragment processing Vaibhav Nagare
@ 2026-07-16 8:47 ` Simon Horman
2026-07-24 14:00 ` [PATCH net v2] " Vaibhav Nagare
0 siblings, 1 reply; 6+ messages in thread
From: Simon Horman @ 2026-07-16 8:47 UTC (permalink / raw)
To: nagarevaibhav
Cc: Simon Horman, davem, kuba, pabeni, edumazet, andrew+netdev,
matvey.kovalev, Pavel.Zhigulin, aelior, manishc, netdev,
linux-kernel, stable, vnagare
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [1]: [PATCH net] qede: Fix NULL pointer dereference in TPA fragment processing ---
commit 437c90c9a6930fba111f83e319f598fb4b80193c
Author: Vaibhav Nagare <nagarevaibhav@gmail.com>
qede: Fix NULL pointer dereference in TPA fragment processing
This commit attempts to fix a NULL pointer dereference under memory pressure
by adding validation in qede_fill_frag_skb() and checking the aggregation
state early in qede_tpa_end() and qede_tpa_cont().
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> index c11e0d8f98aa5..18a3167b35c84 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> @@ -670,13 +670,22 @@ static int qede_fill_frag_skb(struct qede_dev *edev,
> NUM_RX_BDS_MAX];
> struct qede_agg_info *tpa_info = &rxq->tpa_info[tpa_agg_index];
> struct sk_buff *skb = tpa_info->skb;
> + struct page *page = current_bd->data;
>
> if (unlikely(tpa_info->state != QEDE_AGG_STATE_START))
> goto out;
>
> + /* Avoid NULL pointer dereference when under severe memory pressure */
> + if (unlikely(!page)) {
> + DP_NOTICE(edev,
> + "Failed to allocate RX buffer for TPA agg %u\n",
> + tpa_agg_index);
[Severity: Medium]
Since this function executes per-fragment inside the fast-path NAPI poll
loop, could this unconditional DP_NOTICE cause a printk storm under memory
pressure?
If a TPA event has multiple fragments, this might flood the kernel buffer
and potentially cause soft lockups. Should this be a rate-limited print?
> + goto out;
[Severity: Critical]
This is a pre-existing issue, but does jumping to the out label when page is
NULL actually fix the crash, or just delay it?
Looking at the error path:
qede_fill_frag_skb() {
...
out:
tpa_info->state = QEDE_AGG_STATE_ERROR;
qede_recycle_rx_bd_ring(rxq, 1);
...
}
This blindly recycles the poisoned BD via qede_recycle_rx_bd_ring(), placing
the NULL page pointer back into the software ring instead of dropping it.
When a subsequent normal packet arrives, won't the driver panic when
qede_rx_build_skb() calls skb_put_data(skb, page_address(bd->data) + offset)?
The NULL pointer appears to be introduced earlier in qede_tpa_start() when it
copies the mapping and page_offset on failure, but does not copy the data
pointer. Later, qede_tpa_end() calls qede_reuse_page(), pushing this defective
buffer into the active RX ring.
> + }
> +
> /* Add one frag and update the appropriate fields in the skb */
> skb_fill_page_desc(skb, tpa_info->frag_id++,
> - current_bd->data,
> + page,
> current_bd->page_offset + rxq->rx_headroom,
> len_on_bd);
[ ... ]
> @@ -987,6 +1004,10 @@ static int qede_tpa_end(struct qede_dev *edev,
> tpa_info = &rxq->tpa_info[cqe->tpa_agg_index];
> skb = tpa_info->skb;
>
> + /* Drop the packet if TPA start failed */
> + if (unlikely(tpa_info->state != QEDE_AGG_STATE_START || !skb))
> + goto err;
> +
[Severity: Critical]
By adding this early exit to the err label, do we bypass the teardown steps
required to keep the software and hardware rings in sync?
If a TPA aggregation starts successfully but fails later, tpa_info->state is
set to QEDE_AGG_STATE_ERROR. When the hardware subsequently delivers the
TPA_END completion, this early exit will skip the dma_unmap_page() call
for the initial tpa_info->buffer.
It also skips the loop over cqe->len_list:
for (i = 0; i < ARRAY_SIZE(cqe->len_list) && cqe->len_list[i]; i++)
qede_fill_frag_skb(...);
Unlike the fix in qede_tpa_cont() which manually recycles the BDs, skipping
this abandons the BDs used by the hardware. Won't this permanently
desynchronize the software RX consumer index (sw_rx_cons) from the hardware
producer index, leading to out-of-bounds memory accesses?
> if (tpa_info->buffer.page_offset == PAGE_SIZE)
> dma_unmap_page(rxq->dev, tpa_info->buffer.mapping,
> PAGE_SIZE, rxq->data_direction);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net v2] qede: Fix NULL pointer dereference in TPA fragment processing
2026-07-16 8:47 ` Simon Horman
@ 2026-07-24 14:00 ` Vaibhav Nagare
2026-07-24 15:16 ` Markus Elfring
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Vaibhav Nagare @ 2026-07-24 14:00 UTC (permalink / raw)
To: horms, davem, kuba, pabeni, edumazet
Cc: andrew+netdev, matvey.kovalev, Pavel.Zhigulin, aelior, manishc,
netdev, linux-kernel, stable, Vaibhav Nagare
Under memory pressure, the qede driver encounters NULL pointer
dereferences when processing TPA continuation fragments because:
1. qede_fill_frag_skb() does not validate the page pointer before use
2. qede_tpa_end() checks error state AFTER calling qede_fill_frag_skb()
The crash occurs when:
1. System experiences memory pressure (GFP_ATOMIC allocations fail)
2. qede_alloc_rx_buffer() returns -ENOMEM, leaving sw_rx_data->data NULL
3. qede_tpa_start() sets QEDE_AGG_STATE_ERROR on SKB allocation failure
4. Hardware delivers TPA_CONT and TPA_END events for this aggregation
5. qede_tpa_end() calls qede_fill_frag_skb() before checking error state
6. qede_fill_frag_skb() accesses NULL pointer in skb_fill_page_desc()
7. Kernel panics with NULL pointer dereference
Example crash from production system:
BUG: unable to handle kernel NULL pointer dereference at 0x8
RIP: qede_fill_frag_skb+0x96/0x430 [qede]
Call Trace:
qede_rx_int+0xb06/0x1de0
qede_poll+0x2f4/0x6c0
__napi_poll+0x2d/0x130
Observed on HPE Synergy 480 Gen11 running RHEL 8.10
(4.18.0-553.134.1.el8_10.x86_64), but the vulnerable code path
exists in mainline.
Fix by:
1. Adding NULL page validation in qede_fill_frag_skb() before dereferencing
2. Checking error state EARLY in qede_tpa_end() before processing fragments
3. Checking error state in qede_tpa_cont() to skip fragment processing
This allows the system to survive memory pressure by dropping packets
instead of crashing.
Fixes: 55482edc25f0 ("qede: Add slowpath/fastpath support and enable hardware GRO")
Cc: stable@vger.kernel.org
Signed-off-by: Vaibhav Nagare <vnagare@redhat.com>
v2: Addressed AI review feedback from Simon Horman:
- Added net_ratelimit() to prevent printk storm in NAPI fast path
- Fixed NULL buffer recycling: check if page is valid before recycling,
otherwise just consume the BD to prevent NULL from re-entering the ring
- Added proper cleanup in qede_tpa_end() before early exit to prevent
memory leaks and ring desynchronization (DMA unmap + BD recycling)
v1: https://lore.kernel.org/netdev/20260709044704.141507-1-vnagare@redhat.com/
---
drivers/net/ethernet/qlogic/qede/qede_fp.c | 39 ++++++++++++++++++++--
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 33e18bb69774..748b988cbf5d 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -670,13 +670,23 @@ static int qede_fill_frag_skb(struct qede_dev *edev,
NUM_RX_BDS_MAX];
struct qede_agg_info *tpa_info = &rxq->tpa_info[tpa_agg_index];
struct sk_buff *skb = tpa_info->skb;
+ struct page *page = current_bd->data;
if (unlikely(tpa_info->state != QEDE_AGG_STATE_START))
goto out;
+ /* Avoid NULL pointer dereference when under severe memory pressure */
+ if (unlikely(!page)) {
+ if (net_ratelimit())
+ DP_NOTICE(edev,
+ "Failed to allocate RX buffer for TPA agg %u\n",
+ tpa_agg_index);
+ goto out;
+ }
+
/* Add one frag and update the appropriate fields in the skb */
skb_fill_page_desc(skb, tpa_info->frag_id++,
- current_bd->data,
+ page,
current_bd->page_offset + rxq->rx_headroom,
len_on_bd);
@@ -684,7 +694,7 @@ static int qede_fill_frag_skb(struct qede_dev *edev,
/* Incr page ref count to reuse on allocation failure
* so that it doesn't get freed while freeing SKB.
*/
- page_ref_inc(current_bd->data);
+ page_ref_inc(page);
goto out;
}
@@ -698,7 +708,10 @@ static int qede_fill_frag_skb(struct qede_dev *edev,
out:
tpa_info->state = QEDE_AGG_STATE_ERROR;
- qede_recycle_rx_bd_ring(rxq, 1);
+ if (current_bd->data)
+ qede_recycle_rx_bd_ring(rxq, 1);
+ else
+ qede_rx_bd_ring_consume(rxq);
return -ENOMEM;
}
@@ -959,8 +972,16 @@ static inline void qede_tpa_cont(struct qede_dev *edev,
struct qede_rx_queue *rxq,
struct eth_fast_path_rx_tpa_cont_cqe *cqe)
{
+ struct qede_agg_info *tpa_info = &rxq->tpa_info[cqe->tpa_agg_index];
int i;
+ /* Don't process fragments if TPA start failed */
+ if (unlikely(tpa_info->state != QEDE_AGG_STATE_START)) {
+ for (i = 0; i < ARRAY_SIZE(cqe->len_list) && cqe->len_list[i]; i++)
+ qede_recycle_rx_bd_ring(rxq, 1);
+ return;
+ }
+
for (i = 0; i < ARRAY_SIZE(cqe->len_list) && cqe->len_list[i]; i++)
qede_fill_frag_skb(edev, rxq, cqe->tpa_agg_index,
le16_to_cpu(cqe->len_list[i]));
@@ -982,6 +1003,18 @@ static int qede_tpa_end(struct qede_dev *edev,
tpa_info = &rxq->tpa_info[cqe->tpa_agg_index];
skb = tpa_info->skb;
+ /* Drop the packet if TPA start failed */
+ if (unlikely(tpa_info->state != QEDE_AGG_STATE_START || !skb)) {
+ /* Clean up: unmap DMA if needed */
+ if (tpa_info->buffer.page_offset == PAGE_SIZE)
+ dma_unmap_page(rxq->dev, tpa_info->buffer.mapping,
+ PAGE_SIZE, rxq->data_direction);
+ /* Recycle BDs from cqe->len_list to keep ring synchronized */
+ for (i = 0; cqe->len_list[i] && i < ARRAY_SIZE(cqe->len_list); i++)
+ qede_recycle_rx_bd_ring(rxq, 1);
+ goto err;
+ }
+
if (tpa_info->buffer.page_offset == PAGE_SIZE)
dma_unmap_page(rxq->dev, tpa_info->buffer.mapping,
PAGE_SIZE, rxq->data_direction);
--
2.54.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] qede: Fix NULL pointer dereference in TPA fragment processing
2026-07-24 14:00 ` [PATCH net v2] " Vaibhav Nagare
@ 2026-07-24 15:16 ` Markus Elfring
2026-07-28 0:01 ` Jakub Kicinski
2026-07-28 0:03 ` Jakub Kicinski
2 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2026-07-24 15:16 UTC (permalink / raw)
To: Vaibhav Nagare, netdev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: Vaibhav Nagare, stable, LKML, Andrew Lunn, Ariel Elior,
Manish Chopra, Matvey Kovalev, Pavel Zhigulin
…
> Signed-off-by: Vaibhav Nagare <vnagare@redhat.com>
>
> v2: Addressed AI review feedback from Simon Horman:
…
> v1: https://lore.kernel.org/netdev/20260709044704.141507-1-vnagare@redhat.com/
> ---
> drivers/net/ethernet/qlogic/qede/qede_fp.c | 39 ++++++++++++++++++++--
…
Please move patch version descriptions behind the marker line.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc4#n795
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] qede: Fix NULL pointer dereference in TPA fragment processing
2026-07-24 14:00 ` [PATCH net v2] " Vaibhav Nagare
2026-07-24 15:16 ` Markus Elfring
@ 2026-07-28 0:01 ` Jakub Kicinski
2026-07-28 0:03 ` Jakub Kicinski
2 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-07-28 0:01 UTC (permalink / raw)
To: nagarevaibhav
Cc: Jakub Kicinski, horms, davem, pabeni, edumazet, andrew+netdev,
matvey.kovalev, Pavel.Zhigulin, aelior, manishc, netdev,
linux-kernel, stable, vnagare
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
qede: Fix NULL pointer dereference in TPA fragment processing
This patch adds NULL page validation in qede_fill_frag_skb(), moves the
TPA error-state check to run before fragment processing in
qede_tpa_end(), and adds a similar check in qede_tpa_cont(), so that
TPA aggregations whose start failed under memory pressure are dropped
instead of crashing.
Link: https://lore.kernel.org/netdev/20260709044704.141507-1-vnagare@redhat.com/
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> index c11e0d8f98aa..3de6c5a6056c 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> @@ -698,7 +708,10 @@ static int qede_fill_frag_skb(struct qede_dev *edev,
>
> out:
> tpa_info->state = QEDE_AGG_STATE_ERROR;
> - qede_recycle_rx_bd_ring(rxq, 1);
> + if (current_bd->data)
> + qede_recycle_rx_bd_ring(rxq, 1);
> + else
> + qede_rx_bd_ring_consume(rxq);
[Medium]
On the else branch, qede_rx_bd_ring_consume() advances rxq->sw_rx_cons
without a matching produce and without decrementing rxq->filled_buffers.
Every other consume in this driver is paired with either
qede_alloc_rx_buffer() (which increments sw_rx_prod and filled_buffers)
or qede_reuse_page() via qede_recycle_rx_bd_ring() (which produces a
replacement). Here neither happens.
The end-of-NAPI backfill in qede_rx_int() only tops up while
rxq->num_rx_buffers - rxq->filled_buffers is non-zero, so if
filled_buffers is never decremented, the backfill will not replace this
BD.
Does this mean each hit of the NULL-data drop path permanently shrinks
the number of BDs posted to hardware by one, causing the RX ring to
monotonically deplete under the sustained memory pressure this patch is
supposed to make survivable?
Would decrementing rxq->filled_buffers here (or attempting a
replacement allocation) keep the ring in sync with hardware?
>
> return -ENOMEM;
> }
> @@ -964,8 +977,16 @@ static inline void qede_tpa_cont(struct qede_dev *edev,
> struct qede_rx_queue *rxq,
> struct eth_fast_path_rx_tpa_cont_cqe *cqe)
> {
> + struct qede_agg_info *tpa_info = &rxq->tpa_info[cqe->tpa_agg_index];
> int i;
>
> + /* Don't process fragments if TPA start failed */
> + if (unlikely(tpa_info->state != QEDE_AGG_STATE_START)) {
> + for (i = 0; i < ARRAY_SIZE(cqe->len_list) && cqe->len_list[i]; i++)
> + qede_recycle_rx_bd_ring(rxq, 1);
> + return;
> + }
> +
[High]
The other half of the patch adds a guarded fallback in
qede_fill_frag_skb() with the following justification in the v2
changelog:
Fixed NULL buffer recycling: check if page is valid before recycling,
otherwise just consume the BD to prevent NULL from re-entering the
ring
However this new loop calls qede_recycle_rx_bd_ring() unconditionally on
BDs that belong to the same aggregation whose sw_rx_data may have
data == NULL (that is the exact scenario in the commit message: step 2
says qede_alloc_rx_buffer() returned -ENOMEM leaving sw_rx_data->data
NULL, and step 4 says TPA_CONT is then delivered).
qede_recycle_rx_bd_ring() calls qede_reuse_page():
static inline void qede_reuse_page(struct qede_rx_queue *rxq,
struct sw_rx_data *curr_cons)
{
...
*curr_prod = *curr_cons;
...
}
so a NULL data pointer (and possibly a zero mapping) will be copied into
the producer slot and pushed to hardware.
Doesn't this re-introduce exactly the NULL-in-the-ring condition that
the qede_fill_frag_skb() branch above was written to prevent, so the
next TPA aggregation that lands on the recycled slot crashes on the
same NAPI path this commit is trying to protect?
> for (i = 0; i < ARRAY_SIZE(cqe->len_list) && cqe->len_list[i]; i++)
> qede_fill_frag_skb(edev, rxq, cqe->tpa_agg_index,
> le16_to_cpu(cqe->len_list[i]));
> @@ -987,6 +1008,18 @@ static int qede_tpa_end(struct qede_dev *edev,
> tpa_info = &rxq->tpa_info[cqe->tpa_agg_index];
> skb = tpa_info->skb;
>
> + /* Drop the packet if TPA start failed */
> + if (unlikely(tpa_info->state != QEDE_AGG_STATE_START || !skb)) {
> + /* Clean up: unmap DMA if needed */
> + if (tpa_info->buffer.page_offset == PAGE_SIZE)
> + dma_unmap_page(rxq->dev, tpa_info->buffer.mapping,
> + PAGE_SIZE, rxq->data_direction);
> + /* Recycle BDs from cqe->len_list to keep ring synchronized */
> + for (i = 0; cqe->len_list[i] && i < ARRAY_SIZE(cqe->len_list); i++)
> + qede_recycle_rx_bd_ring(rxq, 1);
[High, High]
The two operands of && are reversed here compared to every other loop
over cqe->len_list in this file (including the sibling loop added just
above in qede_tpa_cont()):
for (i = 0; cqe->len_list[i] && i < ARRAY_SIZE(cqe->len_list); i++)
Because && evaluates left-to-right, cqe->len_list[i] is read before the
bound check on i.
ETH_TPA_CQE_END_LEN_LIST_SIZE is 4, and in struct
eth_fast_path_rx_tpa_end_cqe the field immediately following len_list[]
is:
__le16 len_list[ETH_TPA_CQE_END_LEN_LIST_SIZE];
__le16 reserved3[ETH_TPA_CQE_END_LEN_LIST_SIZE];
If firmware delivers a TPA_END with all four len_list entries non-zero,
does iteration i == 4 read reserved3[0] as if it were len_list[4]?
If reserved3[0] happens to be non-zero, does the loop keep iterating
past the array end and call qede_recycle_rx_bd_ring() extra times,
advancing rxq->sw_rx_cons and producing buffers that firmware never
released, silently desynchronising the RX BD ring?
Would swapping the operands to match the pattern used in
qede_tpa_cont() and the two other loops in qede_tpa_end() fix this?
for (i = 0; i < ARRAY_SIZE(cqe->len_list) && cqe->len_list[i]; i++)
Separately, this loop shares the same NULL-buffer concern as the one in
qede_tpa_cont() above: qede_recycle_rx_bd_ring() will copy a NULL
sw_rx_data into the producer slot for any BD whose allocation failed,
which is the exact scenario the commit message describes.
> + goto err;
> + }
> +
> if (tpa_info->buffer.page_offset == PAGE_SIZE)
> dma_unmap_page(rxq->dev, tpa_info->buffer.mapping,
> PAGE_SIZE, rxq->data_direction);
--
pw-bot: cr
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] qede: Fix NULL pointer dereference in TPA fragment processing
2026-07-24 14:00 ` [PATCH net v2] " Vaibhav Nagare
2026-07-24 15:16 ` Markus Elfring
2026-07-28 0:01 ` Jakub Kicinski
@ 2026-07-28 0:03 ` Jakub Kicinski
2 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-07-28 0:03 UTC (permalink / raw)
To: Vaibhav Nagare
Cc: horms, davem, pabeni, edumazet, andrew+netdev, matvey.kovalev,
Pavel.Zhigulin, aelior, manishc, netdev, linux-kernel, stable,
Vaibhav Nagare
Please don't post in reply to previous posting, and...
On Fri, 24 Jul 2026 19:30:44 +0530 Vaibhav Nagare wrote:
> Fix by:
> 1. Adding NULL page validation in qede_fill_frag_skb() before dereferencing
> 2. Checking error state EARLY in qede_tpa_end() before processing fragments
> 3. Checking error state in qede_tpa_cont() to skip fragment processing
>
> This allows the system to survive memory pressure by dropping packets
> instead of crashing.
>
> Fixes: 55482edc25f0 ("qede: Add slowpath/fastpath support and enable hardware GRO")
> Cc: stable@vger.kernel.org
>
.. remove this empty line ..
> Signed-off-by: Vaibhav Nagare <vnagare@redhat.com>
>
.. and the --- separator should be added here, before the change long.
> v2: Addressed AI review feedback from Simon Horman:
> - Added net_ratelimit() to prevent printk storm in NAPI fast path
> - Fixed NULL buffer recycling: check if page is valid before recycling,
> otherwise just consume the BD to prevent NULL from re-entering the ring
> - Added proper cleanup in qede_tpa_end() before early exit to prevent
> memory leaks and ring desynchronization (DMA unmap + BD recycling)
>
> v1: https://lore.kernel.org/netdev/20260709044704.141507-1-vnagare@redhat.com/
I sent out the latest AI complaints.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-28 0:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-09 4:47 [PATCH net] qede: Fix NULL pointer dereference in TPA fragment processing Vaibhav Nagare
2026-07-16 8:47 ` Simon Horman
2026-07-24 14:00 ` [PATCH net v2] " Vaibhav Nagare
2026-07-24 15:16 ` Markus Elfring
2026-07-28 0:01 ` Jakub Kicinski
2026-07-28 0:03 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®