Re: [PATCH net-next v3 15/15] ibmveth: Fix MQ RX poll and shutdown hangs after queue resize
From: Simon Horman <horms@kernel.org>
Date: 2026-07-14 13:15:13
Also in:
netdev
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 --- ibmveth: Fix MQ RX poll and shutdown hangs after queue resize This commit fixes polling hangs in the ibmveth driver after queue resize by skipping over bad correlators instead of retrying endlessly. It also ensures NAPI completes when the interface is down or being disabled.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c index 50a332ab83fd3..eb84b4e5f69fd 100644 --- a/drivers/net/ethernet/ibm/ibmveth.c +++ b/drivers/net/ethernet/ibm/ibmveth.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2959,8 +2978,23 @@ static int ibmveth_poll(struct napi_struct *napi, int budget) __sum16 iph_check = 0; skb = ibmveth_rxq_get_buffer(adapter, queue_index); - if (unlikely(!skb)) - break; + if (unlikely(!skb)) { + if (net_ratelimit()) + netdev_err(netdev,
[Severity: High]
This isn't a bug introduced by this patch, but can the unvalidated packet
lengths and offsets from hardware descriptors trigger an skb_over_panic()
here?
Further down in ibmveth_poll(), the driver reads length and offset directly
from the hardware receive descriptor and passes them to skb_reserve() and
skb_put() on the reused SKB:
int length = ibmveth_rxq_frame_length(adapter, queue_index);
int offset = ibmveth_rxq_frame_offset(adapter, queue_index);
...
} else {
rc = ibmveth_rxq_harvest_buffer(adapter, queue_index, false);
if (unlikely(rc))
break;
skb_reserve(skb, offset);
}
skb_put(skb, length);
If a faulty hypervisor provides an abnormally large length or offset,
skb_put() will extend the tail past the end of the buffer, which triggers an
skb_over_panic() and causes a crash.
Should the driver validate that offset + length is within the known buffer
size before modifying the boundaries?
[ ... ]
quoted hunk ↗ jump to hunk
@@ -3034,8 +3068,11 @@ static int ibmveth_poll(struct napi_struct *napi, int budget) ibmveth_replenish_task(adapter, queue_index); - if (frames_processed == budget) + if (frames_processed == budget) { + if (!netif_running(netdev) || napi_disable_pending(napi)) + napi_complete_done(napi, frames_processed); goto out; + }
[Severity: High] Does calling napi_complete_done() here conflict with the NAPI core contract? If the polling loop exhausts its budget (frames_processed == budget), returning the full budget after calling napi_complete_done() might cause issues with the NAPI state machine. The NAPI core (__napi_poll) will operate on the instance assuming it still holds the NAPIF_STATE_SCHED bit. If napi_disable_pending() is true, the core calls napi_complete(), which will hit a WARN_ON_ONCE. If napi_disable_pending() is false but !netif_running() is true, the core will add the NAPI instance back to the poll_list. Since the driver cleared NAPIF_STATE_SCHED, a subsequent interrupt can trigger another napi_schedule(), causing a double list_add_tail() and potentially leading to list corruption.