Thread (2 messages) 2 messages, 2 authors, 2d ago

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>
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help