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

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

[Sicong Huang]

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.

Resizing the rings via ethtool -G calls efx_realloc_channels(). It
copies each channel, swaps the copies into efx->channel[] and frees the
old channel structures. Nothing on this path cancels the old channel's
filter_work. So the old filter_work can fire after its channel has
been freed. The timer core and delayed_work_timer_fn() then access
the freed delayed_work. efx_filter_rfs_expire() gets the freed channel
back via container_of(), dereferences it and re-arms the timer inside
the freed object.

A possible race is:
CPU0                             CPU1
efx_ethtool_set_ringparam()
efx_realloc_channels()
efx_stop_all()  //NAPI stopped, timer still armed
efx_copy_channel()
swap(efx->channel[i], ...)
efx_remove_channel(old)
kfree(old)  //free here!
                                 old->filter_work.timer fires
                                 delayed_work_timer_fn()
                                 efx_filter_rfs_expire()
                                 channel = container_of(dwork, ...)
                                 channel->rfs_last_expiry  //UAF!
                                 schedule_delayed_work(dwork, 30 * HZ)

Because the work re-arms itself every 30 seconds, this is not a narrow
timing window. One successful ethtool -G after efx_poll() has queued
filter_work is enough to trigger it.

The Siena driver has its own copy of this code in
efx_siena_realloc_channels() and has the same problem.

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@163.com>
---
 drivers/net/ethernet/sfc/efx_channels.c       | 3 +++
 drivers/net/ethernet/sfc/siena/efx_channels.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index f4dc3f3f4416..3773a70e1e7d 100644
--- a/drivers/net/ethernet/sfc/efx_channels.c
+++ b/drivers/net/ethernet/sfc/efx_channels.c
@@ -896,6 +896,9 @@ int efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries)
 	for (i = 0; i < efx->n_channels; i++) {
 		channel = other_channel[i];
 		if (channel && channel->type->copy) {
+#ifdef CONFIG_RFS_ACCEL
+			cancel_delayed_work_sync(&channel->filter_work);
+#endif
 			efx_fini_napi_channel(channel);
 			efx_remove_channel(channel);
 			kfree(channel);
diff --git a/drivers/net/ethernet/sfc/siena/efx_channels.c b/drivers/net/ethernet/sfc/siena/efx_channels.c
index 1fc343598771..d20144f1679a 100644
--- a/drivers/net/ethernet/sfc/siena/efx_channels.c
+++ b/drivers/net/ethernet/sfc/siena/efx_channels.c
@@ -930,6 +930,9 @@ int efx_siena_realloc_channels(struct efx_nic *efx, u32 rxq_entries,
 	for (i = 0; i < efx->n_channels; i++) {
 		channel = other_channel[i];
 		if (channel && channel->type->copy) {
+#ifdef CONFIG_RFS_ACCEL
+			cancel_delayed_work_sync(&channel->filter_work);
+#endif
 			efx_fini_napi_channel(channel);
 			efx_siena_remove_channel(channel);
 			kfree(channel);

base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
-- 
2.34.1

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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@163.com>

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

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

[sicong]

At 2026-10-22 22:36:52, "Edward Cree" <ecree.xilinx@gmail.com> wrote:
>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.

Thanks for the review, that makes sense.

I went with efx_free_channel(). It cancels filter_work synchronously
and then frees the channel, and both efx_realloc_channels() and
efx_fini_channels() now use it instead of calling kfree() directly.
The Siena driver gets the same change.

>(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.)

That seems like a bigger refactor than belongs in a fix for net, so
I've left it out of this patch.

I'll send v2 as a separate thread.

Thanks,
Sicong

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] sfc: missing flush of efx_filter_rfs_work() in
  efx_realloc_channels()
- [Low] The commit fixes a use-after-free in efx_realloc_channels() and
  efx_siena_realloc_channels() that a CAP_NET_ADMIN user can trigger…

--

sfc: fix use-after-free of filter_work in efx_realloc_channels()

The patch cancels each old channel's aRFS filter_work before
efx_realloc_channels() frees the channel. Without this, an ethtool -G ring
resize can leave a self re-arming delayed work pending inside a freed
efx_channel. The same fix is applied to efx_siena_realloc_channels() in the
Siena driver.

> Signed-off-by: Sicong Huang <congei42@163.com>

[Severity: Low]
Should this patch carry Fixes: tags? The only trailer is Signed-off-by. A
CAP_NET_ADMIN user can trigger this use-after-free with ethtool -G. Without a
Fixes: tag, stable and backport tooling cannot tell which trees need the fix.

The work stays pending because of the unconditional re-arm at the end of
efx_filter_rfs_expire() in drivers/net/ethernet/sfc/efx_channels.c:

    schedule_delayed_work(dwork, 30 * HZ);

That re-arm came in with commit 6fbc05e59163 ("sfc: do ARFS expiry work
occasionally even without NAPI poll"). The resize path never cancels that
work.

The Siena copy in drivers/net/ethernet/sfc/siena/efx_channels.c came in with
commit 6e173d3b4af9 ("sfc: Copy shared files needed for Siena (part 1)").

Would something like this be appropriate?

Fixes: 6fbc05e59163 ("sfc: do ARFS expiry work occasionally even without NAPI poll")
Fixes: 6e173d3b4af9 ("sfc: Copy shared files needed for Siena (part 1)")

Another option is to split the patch per driver, so that each half has its
own Fixes: tag.

[ ... ]

> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> --- a/drivers/net/ethernet/sfc/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/efx_channels.c
> @@ -896,6 +896,9 @@ int efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries)
>  	for (i = 0; i < efx->n_channels; i++) {
>  		channel = other_channel[i];
>  		if (channel && channel->type->copy) {
> +#ifdef CONFIG_RFS_ACCEL
> +			cancel_delayed_work_sync(&channel->filter_work);
> +#endif
>  			efx_fini_napi_channel(channel);
>  			efx_remove_channel(channel);
>  			kfree(channel);

[Severity: High]
Does this close the whole window, though? filter_work is not the only
deferred work that holds a pointer to an efx_channel.

When aRFS steers a flow, efx_filter_rfs() claims a slot in
efx->rps_slot[] and schedules req->work. That work runs
efx_filter_rfs_work() in drivers/net/ethernet/sfc/rx_common.c, which
looks up the channel once at entry:

    struct efx_channel *channel = efx_get_channel(efx, req->rxq_index);

It then calls efx->type->filter_insert(). That call can sleep on
efx->filter_sem and the filter table locks, and can wait for MCDI.
After filter_insert() returns, the work goes on using the cached
pointer. It reads channel->rps_flow_id and updates
channel->rfs_filter_count and channel->n_rfs_succeeded or
channel->n_rfs_failed. It may also call
__efx_filter_rfs_expire(channel, ...).

Suppose a request is queued just before efx_stop_all() and is still
inside filter_insert() when efx_realloc_channels() swaps
efx->channel[] and kfree()s the old channel. Those accesses then hit
freed slab memory.

There is a second effect. The counter update lands in the old channel
rather than the copy, so the replacement channel's rfs_filter_count no
longer matches what was installed in the hardware table.

Nothing on the resize path waits for efx->rps_slot[].work. That covers
efx_stop_all(), efx_soft_disable_interrupts() and the teardown loop,
so cancelling filter_work alone does not stop this. Siena has the same
efx_filter_rfs_work() in drivers/net/ethernet/sfc/siena/rx_common.c
and the same gap in efx_siena_realloc_channels().

This patch already fixes the old channel being freed under a work
item. Could it also wait for in-flight aRFS insertion work before the
old channels are freed? One option is to flush each
efx->rps_slot[i].work after NAPI is stopped and before the swap. Or is
there something that already serialises efx_filter_rfs_work() against
the resize that I am missing?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022133927.3860186-1-congei42%40163.com