[PATCH net v2] 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.

Introduce efx_free_channel() to cancel filter_work synchronously
before freeing the channel. Use it in efx_realloc_channels() and
efx_fini_channels(), and in their Siena equivalents. This keeps
cancellation of the per-channel expiry work inside the channel
deallocation API.

Fixes: 3af0f34290f6 ("sfc: replace asynchronous filter operations")
Signed-off-by: Sicong Huang <congei42@xxxxxxx>
---
Changes in v2:
- Introduce efx_free_channel() to cancel filter_work synchronously
  before freeing the channel. Replace the direct kfree() calls in 
  the reallocation and final teardown paths with this helper in 
  both drivers.
- Add the Fixes tag.

Link to v1: https://lore.kernel.org/netdev/20261022133927.3860186-1-congei42@xxxxxxx/

 drivers/net/ethernet/sfc/efx_channels.c       | 12 ++++++++++--
 drivers/net/ethernet/sfc/siena/efx_channels.c | 12 ++++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index f4dc3f3f4416..3e29b892e90c 100644
--- a/drivers/net/ethernet/sfc/efx_channels.c
+++ b/drivers/net/ethernet/sfc/efx_channels.c
@@ -561,6 +561,14 @@ static struct efx_channel *efx_alloc_channel(struct efx_nic *efx, int i)
 	return channel;
 }
 
+static void efx_free_channel(struct efx_channel *channel)
+{
+#ifdef CONFIG_RFS_ACCEL
+	cancel_delayed_work_sync(&channel->filter_work);
+#endif
+	kfree(channel);
+}
+
 int efx_init_channels(struct efx_nic *efx)
 {
 	unsigned int i;
@@ -589,7 +597,7 @@ void efx_fini_channels(struct efx_nic *efx)
 
 	for (i = 0; i < EFX_MAX_CHANNELS; i++)
 		if (efx->channel[i]) {
-			kfree(efx->channel[i]);
+			efx_free_channel(efx->channel[i]);
 			efx->channel[i] = NULL;
 		}
 }
@@ -898,7 +906,7 @@ int efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries)
 		if (channel && channel->type->copy) {
 			efx_fini_napi_channel(channel);
 			efx_remove_channel(channel);
-			kfree(channel);
+			efx_free_channel(channel);
 		}
 	}
 
diff --git a/drivers/net/ethernet/sfc/siena/efx_channels.c b/drivers/net/ethernet/sfc/siena/efx_channels.c
index 1fc343598771..1a8985c153f9 100644
--- a/drivers/net/ethernet/sfc/siena/efx_channels.c
+++ b/drivers/net/ethernet/sfc/siena/efx_channels.c
@@ -563,6 +563,14 @@ static struct efx_channel *efx_alloc_channel(struct efx_nic *efx, int i)
 	return channel;
 }
 
+static void efx_free_channel(struct efx_channel *channel)
+{
+#ifdef CONFIG_RFS_ACCEL
+	cancel_delayed_work_sync(&channel->filter_work);
+#endif
+	kfree(channel);
+}
+
 int efx_siena_init_channels(struct efx_nic *efx)
 {
 	unsigned int i;
@@ -591,7 +599,7 @@ void efx_siena_fini_channels(struct efx_nic *efx)
 
 	for (i = 0; i < EFX_MAX_CHANNELS; i++)
 		if (efx->channel[i]) {
-			kfree(efx->channel[i]);
+			efx_free_channel(efx->channel[i]);
 			efx->channel[i] = NULL;
 		}
 }
@@ -932,7 +940,7 @@ int efx_siena_realloc_channels(struct efx_nic *efx, u32 rxq_entries,
 		if (channel && channel->type->copy) {
 			efx_fini_napi_channel(channel);
 			efx_siena_remove_channel(channel);
-			kfree(channel);
+			efx_free_channel(channel);
 		}
 	}
 

base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
-- 
2.34.1