RE: [Intel-wired-lan] [PATCH net] idpf: disable DIM work before freeing q_vectors

[Salin, Samuel]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf Of
> Myeonghun Pak
> Sent: Monday, July 20, 2026 6:59 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; intel-wired-lan@xxxxxxxxxxxxxxxx
> Cc: Andrew Lunn <andrew+netdev@xxxxxxx>; David S . Miller
> <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub
> Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>;
> netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Ijae Kim
> <ae878000@xxxxxxxxx>
> Subject: [Intel-wired-lan] [PATCH net] idpf: disable DIM work before freeing
> q_vectors
> 
> idpf never drains the Tx/Rx DIM works before freeing the memory they live in.
> tx_dim and rx_dim are embedded in struct idpf_q_vector, they are queued
> from the NAPI poll via net_dim(), and idpf_vport_intr_rel() ends with
> kfree(rsrc->q_vectors).  Nothing in the driver cancels them.
> 
> idpf_tx_dim_work() and idpf_rx_dim_work() then run on freed memory:
> idpf_vport_intr_write_itr() writes the ITR register through q_vector-
> >intr_reg.tx_itr / rx_itr, void __iomem pointers loaded out of the freed
> q_vector.  No configuration is needed to get there --
> IDPF_ITR_IS_DYNAMIC() is defined as (itr_mode) and idpf_vport_alloc()
> initialises both modes to IDPF_ITR_DYNAMIC.
> 
> Draining after idpf_vport_intr_napi_dis_all() is not enough on its own.
> idpf_net_dim() is called from inside the "if (napi_complete_done(napi,
> work_done))" branch of the poll, and
> napi_complete_done() has already cleared NAPIF_STATE_SCHED by then.
> napi_disable_locked() waits only while (val & (NAPIF_STATE_SCHED |
> NAPIF_STATE_NPSVC)), so napi_disable() can return while the poll tail is still
> queueing the work, and a plain cancel_work_sync() would be re-armed behind
> the drain.
> 
> Use disable_work_sync(): schedule_work() on a work with a non-zero disable
> count is dropped by clear_pending_if_disabled() before
> __queue_work() is reached.
> 
> Move idpf_init_dim() to idpf_vport_intr_alloc() so the works are initialised on
> every path that can reach the drain -- the three "goto intr_deinit" sites
> between idpf_vport_intr_init() and
> idpf_vport_intr_ena() get there without the enable side having run.
> Nothing re-enables them: rsrc->q_vectors is freed on every exit from
> idpf_vport_open() and on every idpf_vport_stop(), so the count dies with the
> object.
> 
> It is a race, not a deterministic failure -- net_dim() only schedules once
> DIM_NEVENTS events have accumulated and the profile index changes.
> A KASAN ifup/ifdown loop under load is the way to see it.
> 
> Fixes: c2d548cad150 ("idpf: add TX splitq napi poll support")
> Fixes: 3a8845af66ed ("idpf: add RX splitq napi poll support")
> Cc: <stable@xxxxxxxxxxxxxxx> # see patch description, needs adjustments for
> <= 6.9
> Co-developed-by: Ijae Kim <ae878000@xxxxxxxxx>
> Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
> Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
> ---
> Found by static analysis; not tested on hardware.  Testing by someone at Intel
> with an idpf device would be very welcome.
> 
> disable_work_sync() landed in v6.10, which is newer than the commits this
> fixes, hence the annotation on the stable tag.  A backport to older trees needs
> the work held down another way rather than a cherry-pick; happy to send one
> if the bot reports a failure.
> 
> libwx does the same thing in the same order -- napi_disable() then
> disable_work_sync() on the dim works -- in wx_napi_disable_all().
> 
> Based on v7.2-rc2.
> 2.43.0

Tested-by: Samuel Salin <Samuel.salin@xxxxxxxxx>