Re: [PATCH net v2] net: hip04: fix tx coalesce timer and IRQ teardown races
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Date: 2026-07-10 10:43:51
Also in:
lkml, stable
On 7/10/26 03:57, Fan Wu wrote:
The hip04 remove path frees the TX/RX rings before unregistering the netdev. If the interface is still up, unregister_netdev() then runs .ndo_stop, whose TX reclaim and NAPI poll touch the already-freed DMA ring memory. The TX coalesce timer and the platform IRQ also outlive the netdev private data they dereference. Reorder hip04_remove() so the netdev is unregistered (which runs .ndo_stop synchronously, stopping NAPI and the TX queue) before the rings are freed. Free the devm-managed IRQ explicitly before free_netdev(), so hip04_mac_interrupt() (whose dev_id is the netdev) cannot fire against freed memory: devm would otherwise release it only after .remove returns. hip04_mac_stop() must quiesce both arming sites of the coalesce timer. The NAPI poll arms it, and napi_disable() returns once the poll calls napi_complete_done(), not when the poll function returns, so move that arm before napi_complete_done().
The existing early exits that jump to done do not call napi_complete_done(), so they remain outside the completion-after-arm window this change closes.
this particular sentence is hard to read, as you use AI, would be good to rephrase
The TX xmit path also
s/Tx xmit/Tx/
arms it, and mac_stop() is reached directly from hip04_tx_timeout_task() as well as via .ndo_stop, so use netif_tx_disable() rather than netif_stop_queue() to wait for an in-flight hip04_mac_start_xmit() to finish. The timer is then drained with hrtimer_cancel(). A "closing" flag, checked at the single arming site, guards against a later arm. hip04_tx_timeout_task() restarts the device with mac_stop() + mac_open(); serialize that restart against .ndo_stop with rtnl_lock(), matching the
given the direction to reduce RTNL usage I see no point adding more usage in the driver perhaps netdev_lock() will be sufficient?
netdev core's locking, skip it if the device is no longer running, and emit an error if the restart fails instead of silently leaving it down. This issue was found by an in-house static analysis tool.
Thank you for detailed description, I get from that what the bug is, what is the fix, and agree in principle with all of that.
Fixes: a41ea46a9a12 ("net: hisilicon: new hip04 ethernet driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.5
Signed-off-by: Fan Wu <redacted>