* [PATCH net v3 1/2] sfc: fix use-after-free of filter_work in efx_realloc_channels()
2026-10-31 6:38 [PATCH net v3 0/2] sfc: fix use-after-free of aRFS work on ring resize Sicong Huang
@ 2026-10-31 6:38 ` Sicong Huang
2026-11-01 6:41 ` netdev-bot+sashiko
2026-10-31 6:38 ` [PATCH net v3 2/2] sfc: quiesce aRFS work before copying channels " Sicong Huang
1 sibling, 1 reply; 4+ messages in thread
From: Sicong Huang @ 2026-10-31 6:38 UTC (permalink / raw)
To: netdev
Cc: ecree.xilinx, andrew+netdev, davem, edumazet, kuba, pabeni, kees,
linux-net-drivers, linux-kernel
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@kernel.org/
Signed-off-by: Sicong Huang <congei42@163.com>
---
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
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH net v3 2/2] sfc: quiesce aRFS work before copying channels in efx_realloc_channels()
2026-10-31 6:38 [PATCH net v3 0/2] sfc: fix use-after-free of aRFS work on ring resize Sicong Huang
2026-10-31 6:38 ` [PATCH net v3 1/2] sfc: fix use-after-free of filter_work in efx_realloc_channels() Sicong Huang
@ 2026-10-31 6:38 ` Sicong Huang
1 sibling, 0 replies; 4+ messages in thread
From: Sicong Huang @ 2026-10-31 6:38 UTC (permalink / raw)
To: netdev
Cc: ecree.xilinx, andrew+netdev, davem, edumazet, kuba, pabeni, kees,
linux-net-drivers, linux-kernel
The aRFS insertion worker caches a channel pointer before calling the
sleepable filter_insert() operation. Ring reallocation can copy and
free that channel before the worker updates its flow IDs, filter count
and statistics. Holding a net_device reference does not pin the channel.
The expiry worker can also run between the channel copy and its eventual
cancellation. It then updates the shared rps_flow_id array but only the
old channel's filter count and expiry index, leaving the copy inconsistent.
Quiesce both kinds of work before copying channels. Stopping NAPI alone
does not prevent new insertion requests: efx_ptp_stop() does not wait
for the PTP worker, which can still deliver previously received packets
through netif_receive_skb(). PTP channel probe sets core_index to zero,
and the receive path records that queue on the skb before handing it to
PTP. With aRFS enabled, these packets can still cause insertion requests.
Reject aRFS requests while the net_device is detached, and use
synchronize_net() to wait for ndo_rx_flow_steer() callers that passed the
check before detach. The receive core invokes that callback under
rcu_read_lock().
The same check also rejects requests while the device is detached for
reset, instead of queuing insertion work during reset.
Then flush every insertion work item and synchronously cancel each
channel's expiry work. Initialise the insertion work items once during
NIC structure setup, so even unused slots can safely be flushed. Do not
use rps_slot_map as a completion test: its bits are cleared before the
worker returns, without a release/acquire protocol for copying its state.
The net_device remains detached throughout copying, swapping and cleanup.
The common exit restores expiry on the surviving channels before normal
operation resumes, including allocation failure and probe rollback.
Apply the same changes to Siena.
The insertion-work lifetime and expiry-copy races were reported by
Sashiko and examined by code inspection. Neither the issues 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/179302387159.3134.8640588618973558922@kernel.org/
Link: https://lore.kernel.org/netdev/179310908138.3134.11515982039281965575@kernel.org/
Signed-off-by: Sicong Huang <congei42@163.com>
---
drivers/net/ethernet/sfc/efx_channels.c | 12 ++++++++
drivers/net/ethernet/sfc/efx_common.c | 1 +
drivers/net/ethernet/sfc/rx_common.c | 30 ++++++++++++++++++-
drivers/net/ethernet/sfc/rx_common.h | 2 ++
drivers/net/ethernet/sfc/siena/efx_channels.c | 12 ++++++++
drivers/net/ethernet/sfc/siena/efx_common.c | 1 +
drivers/net/ethernet/sfc/siena/rx_common.c | 30 ++++++++++++++++++-
drivers/net/ethernet/sfc/siena/rx_common.h | 2 ++
8 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index 44943345fa87..da7903205c3a 100644
--- a/drivers/net/ethernet/sfc/efx_channels.c
+++ b/drivers/net/ethernet/sfc/efx_channels.c
@@ -866,6 +866,18 @@ int efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries)
efx_stop_all(efx);
efx_soft_disable_interrupts(efx);
+#ifdef CONFIG_RFS_ACCEL
+ /* efx_copy_channel() copies the aRFS state without rps_mutex, so
+ * all aRFS work must be quiescent before the channels are copied,
+ * not just before the old ones are freed. The detached net_device
+ * rejects new insertion requests, and NAPI is stopped so efx_poll()
+ * cannot re-arm filter_work.
+ */
+ efx_filter_rfs_flush(efx);
+ efx_for_each_channel(channel, efx)
+ cancel_delayed_work_sync(&channel->filter_work);
+#endif
+
/* Clone channels (where possible) */
memset(other_channel, 0, sizeof(other_channel));
for (i = 0; i < efx->n_channels; i++) {
diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c
index 3afaec36642f..08532da706a6 100644
--- a/drivers/net/ethernet/sfc/efx_common.c
+++ b/drivers/net/ethernet/sfc/efx_common.c
@@ -995,6 +995,7 @@ int efx_init_struct(struct efx_nic *efx, struct pci_dev *pci_dev)
#ifdef CONFIG_RFS_ACCEL
mutex_init(&efx->rps_mutex);
spin_lock_init(&efx->rps_hash_lock);
+ efx_filter_rfs_init(efx);
/* Failure to allocate is not fatal, but may degrade ARFS performance */
efx->rps_hash_table = kzalloc_objs(*efx->rps_hash_table,
EFX_ARFS_HASH_TABLE_SIZE);
diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
index fd0a1a8ac2a1..62417c7e907c 100644
--- a/drivers/net/ethernet/sfc/rx_common.c
+++ b/drivers/net/ethernet/sfc/rx_common.c
@@ -895,6 +895,14 @@ static void efx_filter_rfs_work(struct work_struct *data)
netdev_put(req->net_dev, &req->net_dev_tracker);
}
+void efx_filter_rfs_init(struct efx_nic *efx)
+{
+ int slot_idx;
+
+ for (slot_idx = 0; slot_idx < EFX_RPS_MAX_IN_FLIGHT; slot_idx++)
+ INIT_WORK(&efx->rps_slot[slot_idx].work, efx_filter_rfs_work);
+}
+
int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
u16 rxq_index, u32 flow_id)
{
@@ -906,6 +914,10 @@ int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
bool new;
int rc;
+ /* Paired with synchronize_net() in efx_filter_rfs_flush(). */
+ if (!netif_device_present(net_dev))
+ return -ENETDOWN;
+
/* find a free slot */
for (slot_idx = 0; slot_idx < EFX_RPS_MAX_IN_FLIGHT; slot_idx++)
if (!test_and_set_bit(slot_idx, &efx->rps_slot_map))
@@ -986,7 +998,6 @@ int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
/* Queue the request */
req->net_dev = net_dev;
netdev_hold(req->net_dev, &req->net_dev_tracker, GFP_ATOMIC);
- INIT_WORK(&req->work, efx_filter_rfs_work);
req->rxq_index = rxq_index;
req->flow_id = flow_id;
schedule_work(&req->work);
@@ -998,6 +1009,23 @@ int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
return rc;
}
+/* The caller must keep the net_device detached until the channels are safe
+ * for new aRFS requests. Requests can also come from deferred PTP receive
+ * processing after NAPI has stopped.
+ */
+void efx_filter_rfs_flush(struct efx_nic *efx)
+{
+ int slot_idx;
+
+ /* ndo_rx_flow_steer() runs under rcu_read_lock(). Wait for callers
+ * which saw the device before it was detached to finish queueing.
+ */
+ synchronize_net();
+
+ for (slot_idx = 0; slot_idx < EFX_RPS_MAX_IN_FLIGHT; slot_idx++)
+ flush_work(&efx->rps_slot[slot_idx].work);
+}
+
bool __efx_filter_rfs_expire(struct efx_channel *channel, unsigned int quota)
{
bool (*expire_one)(struct efx_nic *efx, u32 flow_id, unsigned int index);
diff --git a/drivers/net/ethernet/sfc/rx_common.h b/drivers/net/ethernet/sfc/rx_common.h
index 75fa84192362..ac4fe0ebab32 100644
--- a/drivers/net/ethernet/sfc/rx_common.h
+++ b/drivers/net/ethernet/sfc/rx_common.h
@@ -103,8 +103,10 @@ struct efx_arfs_rule *efx_rps_hash_add(struct efx_nic *efx,
bool *new);
void efx_rps_hash_del(struct efx_nic *efx, const struct efx_filter_spec *spec);
+void efx_filter_rfs_init(struct efx_nic *efx);
int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
u16 rxq_index, u32 flow_id);
+void efx_filter_rfs_flush(struct efx_nic *efx);
bool __efx_filter_rfs_expire(struct efx_channel *channel, unsigned int quota);
#endif
diff --git a/drivers/net/ethernet/sfc/siena/efx_channels.c b/drivers/net/ethernet/sfc/siena/efx_channels.c
index 388bef4be468..6c2f1492ca58 100644
--- a/drivers/net/ethernet/sfc/siena/efx_channels.c
+++ b/drivers/net/ethernet/sfc/siena/efx_channels.c
@@ -898,6 +898,18 @@ int efx_siena_realloc_channels(struct efx_nic *efx, u32 rxq_entries,
efx_siena_stop_all(efx);
efx_soft_disable_interrupts(efx);
+#ifdef CONFIG_RFS_ACCEL
+ /* efx_copy_channel() copies the aRFS state without rps_mutex, so
+ * all aRFS work must be quiescent before the channels are copied,
+ * not just before the old ones are freed. The detached net_device
+ * rejects new insertion requests, and NAPI is stopped so efx_poll()
+ * cannot re-arm filter_work.
+ */
+ efx_siena_filter_rfs_flush(efx);
+ efx_for_each_channel(channel, efx)
+ cancel_delayed_work_sync(&channel->filter_work);
+#endif
+
/* Clone channels (where possible) */
memset(other_channel, 0, sizeof(other_channel));
for (i = 0; i < efx->n_channels; i++) {
diff --git a/drivers/net/ethernet/sfc/siena/efx_common.c b/drivers/net/ethernet/sfc/siena/efx_common.c
index f91e5f8c0b35..d7615c2394e9 100644
--- a/drivers/net/ethernet/sfc/siena/efx_common.c
+++ b/drivers/net/ethernet/sfc/siena/efx_common.c
@@ -1022,6 +1022,7 @@ int efx_siena_init_struct(struct efx_nic *efx,
#ifdef CONFIG_RFS_ACCEL
mutex_init(&efx->rps_mutex);
spin_lock_init(&efx->rps_hash_lock);
+ efx_siena_filter_rfs_init(efx);
/* Failure to allocate is not fatal, but may degrade ARFS performance */
efx->rps_hash_table = kzalloc_objs(*efx->rps_hash_table,
EFX_ARFS_HASH_TABLE_SIZE);
diff --git a/drivers/net/ethernet/sfc/siena/rx_common.c b/drivers/net/ethernet/sfc/siena/rx_common.c
index 4ceb108fad48..220778a597aa 100644
--- a/drivers/net/ethernet/sfc/siena/rx_common.c
+++ b/drivers/net/ethernet/sfc/siena/rx_common.c
@@ -886,6 +886,14 @@ static void efx_filter_rfs_work(struct work_struct *data)
netdev_put(req->net_dev, &req->net_dev_tracker);
}
+void efx_siena_filter_rfs_init(struct efx_nic *efx)
+{
+ int slot_idx;
+
+ for (slot_idx = 0; slot_idx < EFX_RPS_MAX_IN_FLIGHT; slot_idx++)
+ INIT_WORK(&efx->rps_slot[slot_idx].work, efx_filter_rfs_work);
+}
+
int efx_siena_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
u16 rxq_index, u32 flow_id)
{
@@ -897,6 +905,10 @@ int efx_siena_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
bool new;
int rc;
+ /* Paired with synchronize_net() in efx_siena_filter_rfs_flush(). */
+ if (!netif_device_present(net_dev))
+ return -ENETDOWN;
+
/* find a free slot */
for (slot_idx = 0; slot_idx < EFX_RPS_MAX_IN_FLIGHT; slot_idx++)
if (!test_and_set_bit(slot_idx, &efx->rps_slot_map))
@@ -977,7 +989,6 @@ int efx_siena_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
/* Queue the request */
req->net_dev = net_dev;
netdev_hold(req->net_dev, &req->net_dev_tracker, GFP_ATOMIC);
- INIT_WORK(&req->work, efx_filter_rfs_work);
req->rxq_index = rxq_index;
req->flow_id = flow_id;
schedule_work(&req->work);
@@ -989,6 +1000,23 @@ int efx_siena_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
return rc;
}
+/* The caller must keep the net_device detached until the channels are safe
+ * for new aRFS requests. Requests can also come from deferred PTP receive
+ * processing after NAPI has stopped.
+ */
+void efx_siena_filter_rfs_flush(struct efx_nic *efx)
+{
+ int slot_idx;
+
+ /* ndo_rx_flow_steer() runs under rcu_read_lock(). Wait for callers
+ * which saw the device before it was detached to finish queueing.
+ */
+ synchronize_net();
+
+ for (slot_idx = 0; slot_idx < EFX_RPS_MAX_IN_FLIGHT; slot_idx++)
+ flush_work(&efx->rps_slot[slot_idx].work);
+}
+
bool __efx_siena_filter_rfs_expire(struct efx_channel *channel,
unsigned int quota)
{
diff --git a/drivers/net/ethernet/sfc/siena/rx_common.h b/drivers/net/ethernet/sfc/siena/rx_common.h
index f90a8320d396..2e0fe35c7eb9 100644
--- a/drivers/net/ethernet/sfc/siena/rx_common.h
+++ b/drivers/net/ethernet/sfc/siena/rx_common.h
@@ -94,8 +94,10 @@ struct efx_arfs_rule *efx_siena_rps_hash_find(struct efx_nic *efx,
void efx_siena_rps_hash_del(struct efx_nic *efx,
const struct efx_filter_spec *spec);
+void efx_siena_filter_rfs_init(struct efx_nic *efx);
int efx_siena_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
u16 rxq_index, u32 flow_id);
+void efx_siena_filter_rfs_flush(struct efx_nic *efx);
bool __efx_siena_filter_rfs_expire(struct efx_channel *channel,
unsigned int quota);
#endif
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread