[PATCH net v4] qede: Fix NULL pointer dereference in TPA fragment processing
From: Vaibhav Nagare
Date: Mon Aug 10 2026 - 09:46:39 EST
Under memory pressure, the qede driver encounters NULL pointer
dereferences when processing TPA continuation fragments.
As identified by Jakub Kicinski, commit 8a8633978b84
("qede: Add build_skb() support.") accidentally dropped
the assignment of tpa_info->buffer.data in qede_tpa_start().
When memory pressure causes an SKB allocation failure in qede_tpa_start(),
the driver sets tpa_start_fail = true and attempts to recycle the physical
page later in qede_tpa_end(). However, because buffer.data was left
uninitialized (NULL), qede_reuse_page() pushes a "ghost"
page (valid mapping but NULL data pointer) back into the
active Rx ring. The next time the hardware uses this descriptor,
it passes a NULL page to qede_fill_frag_skb(), causing a kernel
panic.
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 the root cause by:
1. Restoring the tpa_info->buffer.data assignment in qede_tpa_start().
2. Reverting the error recovery block in qede_tpa_end() to rely on
tpa_start_fail, which safely recycles the page without causing
double DMA unmaps.
Additionally, harden the surrounding TPA flow:
3. Add NULL page validation in qede_fill_frag_skb() before dereferencing.
4. Correct bounds checking logic in TPA error loops (evaluating bounds
before reading the array elements to prevent out-of-bounds reads).
5. Check error state early in qede_tpa_end() and qede_tpa_cont() before
processing fragments.
6. Ensure NULL buffer descriptors are consumed rather than recycled, and
correct buffer capacity tracking (rxq->filled_buffers--) to avoid
Rx ring starvation.
Fixes: 8a8633978b84 ("qede: Add build_skb() support.")
Suggested-by: Jakub Kicinski <kuba@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Vaibhav Nagare <vnagare@xxxxxxxxxx>
---
v4: Fix err: label handling as identified by Jakub Kicinski:
- Restored tpa_info->buffer.data assignment in qede_tpa_start(),
which was dropped by commit 8a8633978b84
- Reverted err: label in qede_tpa_end() to use tpa_start_fail flag
instead of buffer.data check (ownership semantics)
- Updated Fixes: tag to 8a8633978b84
- Added Suggested-by: Jakub Kicinski
v3: Addressed additional AI review feedback:
- Fixed NULL pointer recycling in qede_tpa_cont() and qede_tpa_end()
- Fixed array bounds check order in TPA error loops
- Moved version notes after --- per Markus Elfring feedback
- Resent as independent thread per netdev-bot feedback
v2: Addressed AI review feedback from Simon Horman:
- Added net_ratelimit() to prevent printk storm in NAPI fast path
- Fixed NULL buffer recycling in qede_fill_frag_skb()
- Added proper cleanup in qede_tpa_end() early exit path
v1: https://lore.kernel.org/netdev/20260709044704.141507-1-vnagare@xxxxxxxxxx/
drivers/net/ethernet/qlogic/qede/qede_fp.c | 56 ++++++++++++++++++++--
1 file changed, 51 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 33e18bb69774..bf0448b035b4 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,8 +708,12 @@ 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);
+ rxq->filled_buffers--;
+ }
return -ENOMEM;
}
@@ -845,7 +859,7 @@ static void qede_tpa_start(struct qede_dev *edev,
pad, false);
tpa_info->buffer.page_offset = sw_rx_data_cons->page_offset;
tpa_info->buffer.mapping = sw_rx_data_cons->mapping;
-
+ tpa_info->buffer.data = sw_rx_data_cons->data;
if (unlikely(!tpa_info->skb)) {
DP_NOTICE(edev, "Failed to allocate SKB for gro\n");
@@ -959,8 +973,24 @@ 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++) {
+ struct sw_rx_data *rx_bd = &rxq->sw_rx_ring[rxq->sw_rx_cons &
+ NUM_RX_BDS_MAX];
+ if (likely(rx_bd->data)) {
+ qede_recycle_rx_bd_ring(rxq, 1);
+ } else {
+ qede_rx_bd_ring_consume(rxq);
+ rxq->filled_buffers--;
+ }
+ }
+ 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]));
@@ -986,6 +1016,22 @@ static int qede_tpa_end(struct qede_dev *edev,
dma_unmap_page(rxq->dev, tpa_info->buffer.mapping,
PAGE_SIZE, rxq->data_direction);
+ /* Drop the packet if TPA start failed */
+ if (unlikely(tpa_info->state != QEDE_AGG_STATE_START || !skb)) {
+ /* Recycle BDs from cqe->len_list to keep ring synchronized */
+ for (i = 0; i < ARRAY_SIZE(cqe->len_list) && cqe->len_list[i]; i++) {
+ struct sw_rx_data *rx_bd = &rxq->sw_rx_ring[rxq->sw_rx_cons &
+ NUM_RX_BDS_MAX];
+ if (likely(rx_bd->data)) {
+ qede_recycle_rx_bd_ring(rxq, 1);
+ } else {
+ qede_rx_bd_ring_consume(rxq);
+ rxq->filled_buffers--;
+ }
+ }
+ goto err;
+ }
+
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]));
--
2.54.0