* [PATCH net v3 0/2] sfc: fix use-after-free of aRFS work on ring resize
@ 2026-10-31 6:38 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 ` [PATCH net v3 2/2] sfc: quiesce aRFS work before copying channels " Sicong Huang
0 siblings, 2 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
This series fixes aRFS work lifetime and state-copying races when
ethtool -G replaces channel structures. Both sfc and Siena are affected.
Patch 1 keeps expiry cancellation inside efx_free_channel() and re-arms
expiry on the surviving channels, including allocation failure and probe
rollback. Patch 2 prevents new insertion requests while the net_device
is detached, waits for pre-detach receive callbacks and insertion work,
and cancels expiry before copying the channels.
Stopping NAPI alone does not stop deferred PTP receive processing from
submitting aRFS requests. The insertion entry point therefore checks
netif_device_present(), paired with synchronize_net() on the resize
path. All insertion work items are initialised once and flushed without
using the in-flight bitmap as a completion test.
Changes in v3:
- Re-arm expiry after channel replacement, including failure paths,
so it continues without further NAPI polls. (Sashiko, Paolo Abeni)
- Add patch 2 for the insertion-work UAF and expiry-copy race reported
by Sashiko. Cover deferred receive processing as well as NAPI.
- Retain the original Fixes tag: asynchronous expiry already needed
flushing before the later change that added periodic re-arming.
- State the discovery method and testing limits.
Changes in v2:
- Introduce efx_free_channel() to encapsulate expiry cancellation in
the channel deallocation helper. (Edward Cree)
- Add the Fixes tag.
Testing: the modified translation units build with GCC 11 on x86_64,
W=1 and CONFIG_RFS_ACCEL both enabled and disabled. No hardware testing
or runtime reproduction has been performed.
v2: https://lore.kernel.org/netdev/20261023134214.322796-1-congei42@163.com/
v1: https://lore.kernel.org/netdev/20261022133927.3860186-1-congei42@163.com/
Sicong Huang (2):
sfc: fix use-after-free of filter_work in efx_realloc_channels()
sfc: quiesce aRFS work before copying channels in
efx_realloc_channels()
drivers/net/ethernet/sfc/efx_channels.c | 35 +++++++++++++++++--
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 | 35 +++++++++++++++++--
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, 130 insertions(+), 6 deletions(-)
base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [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
* Re: [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 1/2] sfc: fix use-after-free of filter_work in efx_realloc_channels() Sicong Huang
@ 2026-11-01 6:41 ` netdev-bot+sashiko
0 siblings, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-11-01 6:41 UTC (permalink / raw)
To: congei42
Cc: netdev, ecree.xilinx, andrew+netdev, davem, edumazet, kuba,
pabeni, kees, linux-net-drivers, linux-kernel
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 1 · Low: 0
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] Applied alone, the patch "sfc: fix use-after-free of filter_work
in efx_realloc_channels()" only cancels channel->filter_work in the…
- [Medium] Applied alone, the patch "sfc: fix use-after-free of
filter_work in efx_realloc_channels()" leaves the old channels' aRFS…
--
sfc: fix use-after-free of filter_work in efx_realloc_channels()
This adds efx_free_channel(), which cancels the per-channel aRFS expiry
work synchronously before freeing a channel. It is used in
efx_realloc_channels() and efx_fini_channels(), and the expiry work is
re-armed at the common exit of efx_realloc_channels() for channels that
still have filters. The same changes are applied to Siena.
Link: https://lore.kernel.org/netdev/179310908138.3134.11515982039281965575@kernel.org/
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> index f4dc3f3f4416c..44943345fa87a 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);
> +}
[Severity: High]
Is waiting for filter_work enough before this kfree()? The per-NIC aRFS
insertion works in efx->rps_slot[] can also hold a pointer to this
channel. efx_filter_rfs() queues them with schedule_work().
efx_filter_rfs_work() in rx_common.c looks up the channel when it
starts. It then sleeps in the MCDI insert before it touches the channel:
drivers/net/ethernet/sfc/rx_common.c:efx_filter_rfs_work() {
struct efx_channel *channel = efx_get_channel(efx, req->rxq_index);
...
rc = efx->type->filter_insert(efx, &req->spec, true);
...
if (rc >= 0) {
mutex_lock(&efx->rps_mutex);
if (channel->rps_flow_id[rc] == RPS_FLOW_ID_INVALID)
channel->rfs_filter_count++;
...
channel->n_rfs_succeeded++;
} else {
...
channel->n_rfs_failed++;
...
__efx_filter_rfs_expire(channel, min(channel->rfs_filter_count,
100u));
}
...
}
None of efx_device_detach_sync(), efx_stop_all() or
efx_soft_disable_interrupts() flushes these works.
Can efx_realloc_channels() call efx_free_channel() on an old channel
while one of these works is still blocked in filter_insert()? If so, the
work writes into the freed channel when it resumes.
The failure path also runs an expiry pass on the freed channel, which is
the problem this patch is meant to close. Siena has the same pattern, in
its efx_filter_rfs_work() and efx_siena_realloc_channels().
The next patch in this series, "sfc: quiesce aRFS work before copying
channels in efx_realloc_channels()", appears to fix this:
- It adds a netif_device_present() check to efx_filter_rfs() and
efx_siena_filter_rfs().
- It adds efx_filter_rfs_flush(), which calls synchronize_net() and
then flush_work() on each rps_slot[].work. This runs before the
channels are copied.
Could that be folded into this patch so it is correct on its own?
[ ... ]
> @@ -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
[Severity: Medium]
Can the rfs_filter_count checked here be stale on the copies?
The copies come from efx_copy_channel(). It does a plain struct copy
without taking rps_mutex:
drivers/net/ethernet/sfc/efx_channels.c:efx_copy_channel() {
...
*channel = *old_channel;
...
}
This copies rfs_filter_count, rfs_expire_index and rfs_last_expiry. The
copy also shares the rps_flow_id pointer with the old channel.
With this patch, the old channel's filter_work is only cancelled in
efx_free_channel(), in the out: loop above. By then the clone, the swap
and efx_probe_channel() on every copy have already run.
Until that cancel, __efx_filter_rfs_expire() and efx_filter_rfs_work()
can still update old->rfs_filter_count and the shared rps_flow_id[]
under rps_mutex:
drivers/net/ethernet/sfc/rx_common.c:__efx_filter_rfs_expire() {
...
channel->rps_flow_id[index] = RPS_FLOW_ID_INVALID;
channel->rfs_filter_count--;
...
}
So the copy's count could be too high after an expiry on the old
channel. It could also be too low after an insertion, and later wrap.
Suppose a copy's count ends up at 0 while rps_flow_id[] still holds
valid entries. Would this loop skip that channel, so its filters never
expire? That seems to be the case this block was added for.
efx_siena_realloc_channels() has the same ordering.
The next patch in this series, "sfc: quiesce aRFS work before copying
channels in efx_realloc_channels()", seems to fix this too. Before the
clone loop, it calls efx_filter_rfs_flush() and then
cancel_delayed_work_sync() on each channel's filter_work. It does this
in both efx_realloc_channels() and efx_siena_realloc_channels().
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261031063823.876645-1-congei42%40163.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-01 6:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®