Re: [PATCH net v2] net: hip04: fix tx coalesce timer and IRQ teardown races

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

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 <fanwu01@zju.edu.cn>