From: Jakub Kicinski kuba@kernel.org
[ Upstream commit 4c03592689bc19df9deda7a33d56c6ac0cec8651 ]
Commit e20dfbad8aab ("net: fix napi_consume_skb() with alien skbs") added a skb->cpu check to napi_consume_skb(), before the point where napi_consume_skb() validated skb is not NULL.
Add an explicit check to the early exit condition.
Reviewed-by: Eric Dumazet edumazet@google.com Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Summary Analysis
### 1. COMMIT MESSAGE ANALYSIS - **Purpose**: Fixes a regression introduced by commit e20dfbad8aab - **Problem**: The prior commit added `skb->alloc_cpu` check *before* the point where NULL was being handled - **Keywords**: "restore" indicates fixing a regression; explicitly references the bug-introducing commit - **Tags**: Reviewed-by Eric Dumazet (top networking maintainer), Signed-off-by Jakub Kicinski (net maintainer)
### 2. CODE CHANGE ANALYSIS
**Before the bug (e20dfbad8aab~1)**: ```c void napi_consume_skb(struct sk_buff *skb, int budget) { if (unlikely(!budget)) { dev_consume_skb_any(skb); // Safe with NULL return; } if (!skb_unref(skb)) // skb_unref() handles NULL! return; ... ```
**After e20dfbad8aab introduced the bug**: ```c if (skb->alloc_cpu != smp_processor_id() ...) // CRASH if skb==NULL ```
**The fix adds `|| !skb`** to redirect NULL skbs to `dev_consume_skb_any()` which safely handles NULL (verified via call chain: `dev_consume_skb_any` → `dev_kfree_skb_any_reason` → `sk_skb_reason_drop` → `skb_unref` which has explicit NULL check).
**Technical Mechanism**: The bug is a NULL pointer dereference. When `budget != 0` and `skb == NULL`, the code would try to access `skb->alloc_cpu`, causing a kernel oops/crash.
### 3. CLASSIFICATION - **Type**: Regression fix for NULL pointer dereference - **Severity**: HIGH - kernel crash/oops - **Category**: Core networking bug fix (not device ID, quirk, or DT)
### 4. SCOPE AND RISK ASSESSMENT - **Lines changed**: 1 (adds `|| !skb` to existing condition) - **Files touched**: 1 (`net/core/skbuff.c`) - **Complexity**: Trivial - single condition addition - **Risk**: VERY LOW - adds defensive NULL check to existing early-exit path - **Subsystem**: Core networking (`napi_consume_skb` is used by many drivers)
### 5. USER IMPACT - **Affected users**: Anyone using networking with drivers that call `napi_consume_skb(NULL, non_zero_budget)` - **Severity if hit**: Kernel crash/oops - **Scope**: Core networking path = HIGH impact potential
### 6. STABILITY INDICATORS - Reviewed-by: Eric Dumazet (authoritative networking maintainer) - Signed-off-by: Jakub Kicinski (net maintainer) - Obviously correct fix (adding NULL check before dereference)
### 7. DEPENDENCY CHECK - **Depends on**: Commit e20dfbad8aab must be present (the bug- introducing commit) - This fix should be backported to any stable tree that contains e20dfbad8aab
### CONCLUSION
This is a textbook stable backport candidate:
1. ✅ **Obviously correct**: Adding NULL check before pointer dereference 2. ✅ **Fixes real bug**: NULL pointer dereference → kernel crash 3. ✅ **Important issue**: Kernel oops in core networking path 4. ✅ **Small and contained**: Single character addition (`|| !skb`) 5. ✅ **No new features**: Pure regression fix 6. ✅ **Expert reviewed**: Eric Dumazet and Jakub Kicinski (net maintainers) 7. ✅ **Restores previous behavior**: Explicit goal stated in commit message
The fix is minimal, surgical, and addresses a serious crash in core networking code. The dependency on e20dfbad8aab is the only consideration - this fix must accompany that commit in any stable tree.
**YES**
net/core/skbuff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 6be01454f262a..9f9ea86620ace 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -1458,7 +1458,7 @@ void napi_skb_free_stolen_head(struct sk_buff *skb) void napi_consume_skb(struct sk_buff *skb, int budget) { /* Zero budget indicate non-NAPI context called us, like netpoll */ - if (unlikely(!budget)) { + if (unlikely(!budget || !skb)) { dev_consume_skb_any(skb); return; }