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

[Sicong Huang]

With CONFIG_RFS_ACCEL, efx_poll() queues the per-channel aRFS expiry
work. The work also re-arms itself every 30 seconds so that filters
can expire even when the channel receives no more NAPI polls.

efx_realloc_channels(), called by ethtool -G, copies the channels and
frees the old structures without cancelling their filter_work. A queued
or running expiry callback can therefore access the freed channel and
re-arm a timer in freed memory. The work lifetime problem dates back to
the introduction of asynchronous filter operations; the later periodic
re-arm makes it possible even without recent receive activity.

Introduce efx_free_channel() to cancel filter_work synchronously before
freeing its channel. Use it for ring reallocation and final channel
teardown, keeping this requirement inside the deallocation helper.

The copied channels retain the installed filters but start with an idle
filter_work. Re-arm expiry on the channels that still have filters at
the common reallocation exit, so expiry continues without another NAPI
poll. This also covers allocation failure and probe rollback, when the
original channels remain in use. Apply the same changes to Siena.

Identified by code inspection. The missing expiry re-arm was pointed
out by Sashiko review and Paolo Abeni. Neither the issue nor the fix
has been tested on hardware.
Compile-tested on x86_64 with CONFIG_RFS_ACCEL enabled and disabled.

Fixes: 3af0f34290f6 ("sfc: replace asynchronous filter operations")
Link: https://lore.kernel.org/netdev/179310908138.3134.11515982039281965575@xxxxxxxxxx/
Signed-off-by: Sicong Huang <congei42@xxxxxxx>
---
 drivers/net/ethernet/sfc/efx_channels.c       | 23 +++++++++++++++++--
 drivers/net/ethernet/sfc/siena/efx_channels.c | 23 +++++++++++++++++--
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index f4dc3f3f4416..44943345fa87 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,10 +906,21 @@ 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);
 		}
 	}
 
+#ifdef CONFIG_RFS_ACCEL
+	/* Re-arm the expiry work on the channels now in use: the copies on
+	 * success, the originals after a rollback.  The copies inherit the
+	 * aRFS filters, so they must inherit the expiry too, or a channel
+	 * that gets no further NAPI polls would never expire its filters.
+	 */
+	efx_for_each_channel(channel, efx)
+		if (channel->rfs_filter_count)
+			schedule_delayed_work(&channel->filter_work, 30 * HZ);
+#endif
+
 	efx->ptp_data = ptp_data;
 	rc2 = efx_soft_enable_interrupts(efx);
 	if (rc2) {
diff --git a/drivers/net/ethernet/sfc/siena/efx_channels.c b/drivers/net/ethernet/sfc/siena/efx_channels.c
index 1fc343598771..388bef4be468 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,10 +940,21 @@ 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);
 		}
 	}
 
+#ifdef CONFIG_RFS_ACCEL
+	/* Re-arm the expiry work on the channels now in use: the copies on
+	 * success, the originals after a rollback.  The copies inherit the
+	 * aRFS filters, so they must inherit the expiry too, or a channel
+	 * that gets no further NAPI polls would never expire its filters.
+	 */
+	efx_for_each_channel(channel, efx)
+		if (channel->rfs_filter_count)
+			schedule_delayed_work(&channel->filter_work, 30 * HZ);
+#endif
+
 	rc2 = efx_soft_enable_interrupts(efx);
 	if (rc2) {
 		rc = rc ? rc : rc2;
-- 
2.34.1