Re: [PATCH net] sfc: fix use-after-free of filter_work in efx_realloc_channels()

[Edward Cree]

On 22/10/2026 14:39, Sicong Huang wrote:
> With CONFIG_RFS_ACCEL, each channel has a delayed work item,
> channel->filter_work, that expires aRFS filters. efx_poll() queues it
> with mod_delayed_work(), and efx_filter_rfs_expire() always re-arms it
> with schedule_delayed_work(dwork, 30 * HZ). Once efx_poll() has queued
> it, the work is therefore always pending, queued or running. It is
> only cancelled in efx_remove_filters(), which is not called on the ring
> resize path.
...
> Fix this in both drivers by calling cancel_delayed_work_sync() on
> filter_work before freeing an unused channel. The work does not take
> the RTNL. It only trylocks efx->rps_mutex, and the filter table locks
> it takes are never held while waiting for the RTNL, so waiting for it
> under the RTNL cannot deadlock. On the copy failure and rollback paths,
> the freed channels are new copies whose filter_work was never queued,
> so the cancel does nothing there.
> 
> Signed-off-by: Sicong Huang <congei42@xxxxxxx>

The analysis looks correct, but I wonder if rather than placing the
 cancel_delayed_work_sync() inline in efx_realloc_channels(), it should
 be done by efx_remove_channel() (or rolled up with the kfree() into a
 new efx_free_channel(), since it's undoing efx_alloc_channel() rather
 than efx_probe_channel()), to reduce the risk of the same race popping
 up elsewhere.  Consumers of the channels API should not need to know
 that they have to do this 'magic' cancel to safely free a channel,
 that's a layering violation.

(Actually, it's a layering violation that filter_work even lives in the
 efx_channel and not its efx_rx_queue, but I never had time to fix that
 and am now not likely to — I commend the idea to future maintainers.)

-ed