* [PATCH net] netdevsim: tc: serialize access to nsim_block_cb_list
@ 2026-07-19 17:50 Weiming Shi
2026-07-24 14:23 ` Simon Horman
2026-07-26 12:03 ` [PATCH net v2] net: flow_offload: serialize driver callback lists Weiming Shi
0 siblings, 2 replies; 4+ messages in thread
From: Weiming Shi @ 2026-07-19 17:50 UTC (permalink / raw)
To: Jakub Kicinski, Andrew Lunn, David S . Miller, Eric Dumazet,
Paolo Abeni
Cc: Pablo Neira Ayuso, netdev, linux-kernel, Xiang Mei, Weiming Shi,
stable
nsim_setup_tc() passes a single global nsim_block_cb_list, shared by every
netdevsim device, to flow_block_cb_setup_simple(). That helper does
list_add_tail()/list_del() on the list on block bind/unbind and walks it
in flow_block_cb_is_busy(); it takes no lock of its own and relies on the
caller for serialization.
The tc control path calls ndo_setup_tc(TC_SETUP_BLOCK) under rtnl, but the
nf_tables hardware offload path reaches it under the per-netns nftables
commit_mutex only. Two nft transactions committing offload chains from
different netns thus run flow_block_cb_setup_simple() on the same list
concurrently, corrupting it and freeing a flow_block_cb that the other CPU
still walks:
list_del corruption. prev->next should be ffff88801f18ff00, but was ffff88801de82b00.
kernel BUG at lib/list_debug.c:62!
RIP: 0010:__list_del_entry_valid_or_report (lib/list_debug.c:62)
flow_block_cb_setup_simple (net/core/flow_offload.c:369)
nsim_setup_tc (drivers/net/netdevsim/tc.c)
nft_block_offload_cmd (net/netfilter/nf_tables_offload.c:394)
nft_flow_rule_offload_commit (net/netfilter/nf_tables_offload.c:585)
nf_tables_commit (net/netfilter/nf_tables_api.c:10490)
nfnetlink_rcv_batch (net/netfilter/nfnetlink.c:577)
nfnetlink_rcv (net/netfilter/nfnetlink.c:649)
netlink_unicast (net/netlink/af_netlink.c:1314)
netlink_sendmsg (net/netlink/af_netlink.c:1889)
__sys_sendto (net/socket.c:729)
With KASAN the same race is reported as a slab-use-after-free read of the
freed flow_block_cb (kmalloc-192) in flow_block_cb_setup_simple(). The
trace above is from a 6.12.y reproduction.
Serialize the list with a mutex. ndo_setup_tc(TC_SETUP_BLOCK) always runs
in process context, so sleeping on the mutex is fine.
Fixes: 955bcb6ea0df ("drivers: net: use flow block API")
Reported-by: Xiang Mei <xmei5@asu.edu>
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
drivers/net/netdevsim/tc.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/netdevsim/tc.c b/drivers/net/netdevsim/tc.c
index a415e02a6df1..30dd7f924b71 100644
--- a/drivers/net/netdevsim/tc.c
+++ b/drivers/net/netdevsim/tc.c
@@ -72,11 +72,13 @@ static int nsim_setup_tc_ets(struct net_device *dev,
}
static LIST_HEAD(nsim_block_cb_list);
+static DEFINE_MUTEX(nsim_block_cb_lock);
int
nsim_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data)
{
struct netdevsim *ns = netdev_priv(dev);
+ int err;
switch (type) {
case TC_SETUP_QDISC_TAPRIO:
@@ -84,10 +86,13 @@ nsim_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data)
case TC_SETUP_QDISC_ETS:
return nsim_setup_tc_ets(dev, type_data);
case TC_SETUP_BLOCK:
- return flow_block_cb_setup_simple(type_data,
- &nsim_block_cb_list,
- nsim_setup_tc_block_cb,
- ns, ns, true);
+ mutex_lock(&nsim_block_cb_lock);
+ err = flow_block_cb_setup_simple(type_data,
+ &nsim_block_cb_list,
+ nsim_setup_tc_block_cb,
+ ns, ns, true);
+ mutex_unlock(&nsim_block_cb_lock);
+ return err;
case TC_SETUP_FT:
return 0;
default:
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] netdevsim: tc: serialize access to nsim_block_cb_list
2026-07-19 17:50 [PATCH net] netdevsim: tc: serialize access to nsim_block_cb_list Weiming Shi
@ 2026-07-24 14:23 ` Simon Horman
2026-07-26 8:26 ` Weiming Shi
2026-07-26 12:03 ` [PATCH net v2] net: flow_offload: serialize driver callback lists Weiming Shi
1 sibling, 1 reply; 4+ messages in thread
From: Simon Horman @ 2026-07-24 14:23 UTC (permalink / raw)
To: Weiming Shi
Cc: Jakub Kicinski, Andrew Lunn, David S . Miller, Eric Dumazet,
Paolo Abeni, Pablo Neira Ayuso, netdev, linux-kernel, Xiang Mei,
stable
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/20260719175028.988301-2-bestswngs@gmail.com?part=1
---
--- Patch [1]: [PATCH net] netdevsim: tc: serialize access to nsim_block_cb_list ---
commit 27d18d348023b46d852b44b24e6f4ca81ab39bf3
Author: Weiming Shi <bestswngs@gmail.com>
netdevsim: tc: serialize access to nsim_block_cb_list
This commit attempts to fix a list corruption and slab-use-after-free race
condition between the nftables hardware offload path and the TC control
path. It adds a local mutex to netdevsim to serialize access to the global
nsim_block_cb_list during flow_block_cb_setup_simple().
> diff --git a/drivers/net/netdevsim/tc.c b/drivers/net/netdevsim/tc.c
> index a415e02a6df1e..30dd7f924b716 100644
> --- a/drivers/net/netdevsim/tc.c
> +++ b/drivers/net/netdevsim/tc.c
> @@ -72,11 +72,13 @@ static int nsim_setup_tc_ets(struct net_device *dev,
> }
>
> static LIST_HEAD(nsim_block_cb_list);
> +static DEFINE_MUTEX(nsim_block_cb_lock);
>
> int
> nsim_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data)
> {
> struct netdevsim *ns = netdev_priv(dev);
> + int err;
>
> switch (type) {
> case TC_SETUP_QDISC_TAPRIO:
[ ... ]
> @@ -84,10 +86,13 @@ nsim_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data)
> case TC_SETUP_QDISC_ETS:
> return nsim_setup_tc_ets(dev, type_data);
> case TC_SETUP_BLOCK:
> - return flow_block_cb_setup_simple(type_data,
> - &nsim_block_cb_list,
> - nsim_setup_tc_block_cb,
> - ns, ns, true);
> + mutex_lock(&nsim_block_cb_lock);
Simon says: I'm wondering if you could take a look at the issue flagged
below as it seems to undermine the fix proposed by this
patch.
[Severity: High]
This isn't a bug introduced by this patch, but will the new nsim_block_cb_lock
actually fully serialize access to the list?
If tcf_block_bind() fails in the core TC stack, its error unroll path
directly executes list_del(&block_cb->driver_list) while holding only
rtnl_lock.
If this error unroll happens concurrently with an nftables transaction that
holds the commit_mutex and the newly introduced nsim_block_cb_lock, the core
TC stack will modify nsim_block_cb_list completely bypassing the driver's
local lock.
Could this still lead to list corruption and slab-use-after-free bugs when the
concurrent nftables transaction walks the mutating list?
> + err = flow_block_cb_setup_simple(type_data,
Simon says: I think the problem flagged below can be considered
in the context of possible follow-up.
[Severity: High]
This is a pre-existing issue, but does adding a local lock in netdevsim mask
the underlying systemic race?
flow_block_cb_setup_simple() modifies driver-provided lists directly without
internal locking. Since the nftables hardware offload path runs without
rtnl_lock, unprivileged users can configure nftables concurrently and trigger
list corruption.
While this patch addresses the issue for netdevsim locally, won't this same
vulnerability persist across other hardware drivers (e.g., cxgb4, prestera,
i40e) that use TC_SETUP_BLOCK with statically allocated global lists?
> + &nsim_block_cb_list,
> + nsim_setup_tc_block_cb,
> + ns, ns, true);
> + mutex_unlock(&nsim_block_cb_lock);
> + return err;
> case TC_SETUP_FT:
> return 0;
> default:
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] netdevsim: tc: serialize access to nsim_block_cb_list
2026-07-24 14:23 ` Simon Horman
@ 2026-07-26 8:26 ` Weiming Shi
0 siblings, 0 replies; 4+ messages in thread
From: Weiming Shi @ 2026-07-26 8:26 UTC (permalink / raw)
To: Simon Horman
Cc: Jakub Kicinski, Andrew Lunn, David S . Miller, Eric Dumazet,
Paolo Abeni, Pablo Neira Ayuso, netdev, linux-kernel, Xiang Mei,
stable
Simon Horman <horms@kernel.org> 于2026年7月24日周五 22:23写道：
>
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev/#/patchset/20260719175028.988301-2-bestswngs@gmail.com?part=1
> ---
> --- Patch [1]: [PATCH net] netdevsim: tc: serialize access to nsim_block_cb_list ---
> commit 27d18d348023b46d852b44b24e6f4ca81ab39bf3
> Author: Weiming Shi <bestswngs@gmail.com>
>
> netdevsim: tc: serialize access to nsim_block_cb_list
>
> This commit attempts to fix a list corruption and slab-use-after-free race
> condition between the nftables hardware offload path and the TC control
> path. It adds a local mutex to netdevsim to serialize access to the global
> nsim_block_cb_list during flow_block_cb_setup_simple().
>
> > diff --git a/drivers/net/netdevsim/tc.c b/drivers/net/netdevsim/tc.c
> > index a415e02a6df1e..30dd7f924b716 100644
> > --- a/drivers/net/netdevsim/tc.c
> > +++ b/drivers/net/netdevsim/tc.c
> > @@ -72,11 +72,13 @@ static int nsim_setup_tc_ets(struct net_device *dev,
> > }
> >
> > static LIST_HEAD(nsim_block_cb_list);
> > +static DEFINE_MUTEX(nsim_block_cb_lock);
> >
> > int
> > nsim_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data)
> > {
> > struct netdevsim *ns = netdev_priv(dev);
> > + int err;
> >
> > switch (type) {
> > case TC_SETUP_QDISC_TAPRIO:
> [ ... ]
> > @@ -84,10 +86,13 @@ nsim_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data)
> > case TC_SETUP_QDISC_ETS:
> > return nsim_setup_tc_ets(dev, type_data);
> > case TC_SETUP_BLOCK:
> > - return flow_block_cb_setup_simple(type_data,
> > - &nsim_block_cb_list,
> > - nsim_setup_tc_block_cb,
> > - ns, ns, true);
> > + mutex_lock(&nsim_block_cb_lock);
>
> Simon says: I'm wondering if you could take a look at the issue flagged
> below as it seems to undermine the fix proposed by this
> patch.
>
> [Severity: High]
> This isn't a bug introduced by this patch, but will the new nsim_block_cb_lock
> actually fully serialize access to the list?
>
> If tcf_block_bind() fails in the core TC stack, its error unroll path
> directly executes list_del(&block_cb->driver_list) while holding only
> rtnl_lock.
>
> If this error unroll happens concurrently with an nftables transaction that
> holds the commit_mutex and the newly introduced nsim_block_cb_lock, the core
> TC stack will modify nsim_block_cb_list completely bypassing the driver's
> local lock.
>
> Could this still lead to list corruption and slab-use-after-free bugs when the
> concurrent nftables transaction walks the mutating list?
>
> > + err = flow_block_cb_setup_simple(type_data,
>
> Simon says: I think the problem flagged below can be considered
> in the context of possible follow-up.
>
> [Severity: High]
> This is a pre-existing issue, but does adding a local lock in netdevsim mask
> the underlying systemic race?
>
> flow_block_cb_setup_simple() modifies driver-provided lists directly without
> internal locking. Since the nftables hardware offload path runs without
> rtnl_lock, unprivileged users can configure nftables concurrently and trigger
> list corruption.
>
> While this patch addresses the issue for netdevsim locally, won't this same
> vulnerability persist across other hardware drivers (e.g., cxgb4, prestera,
> i40e) that use TC_SETUP_BLOCK with statically allocated global lists?
>
> > + &nsim_block_cb_list,
> > + nsim_setup_tc_block_cb,
> > + ns, ns, true);
> > + mutex_unlock(&nsim_block_cb_lock);
> > + return err;
> > case TC_SETUP_FT:
> > return 0;
> > default:
Hi Simon,
Thanks for the feedback. That makes sense, and I agree with the
concern. I'll work on a v2 patch to address this.
Thanks,
Weiming
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH net v2] net: flow_offload: serialize driver callback lists
2026-07-19 17:50 [PATCH net] netdevsim: tc: serialize access to nsim_block_cb_list Weiming Shi
2026-07-24 14:23 ` Simon Horman
@ 2026-07-26 12:03 ` Weiming Shi
1 sibling, 0 replies; 4+ messages in thread
From: Weiming Shi @ 2026-07-26 12:03 UTC (permalink / raw)
To: Claudiu Manoil, Vladimir Oltean, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Pablo Neira Ayuso, Jozsef Kadlecsik, Jamal Hadi Salim, Cong Wang,
Jiri Pirko
Cc: netdev, linux-kernel, netfilter-devel, coreteam,
syzbot+5a66db916cdde0dbcc1c, Xiang Mei, stable
Drivers keep flow_block_cb objects on callback lists which are often
shared globally. TC setup normally runs under RTNL, but nftables invokes
ndo_setup_tc() while holding only its per-netns commit mutex. Concurrent
transactions in different netns can therefore corrupt a shared driver
callback list.
The same list can also be changed after ndo_setup_tc() returns. If filter
replay fails while binding a TC block, tcf_block_bind() removes and frees
callbacks from its error path without synchronizing against an nftables
transaction walking the driver list.
Add a flow-offload callback mutex and hold it across direct and indirect
setup, callback ownership transfer and rollback. Move indirect-unregister
list removal into the flow-offload core, and keep indirect-device
registration transactional on setup failure. When TC must reject an
indirect bind after callback installation, unwind only the callbacks
created by that bind so existing callbacks remain intact.
Initialize the auxiliary callback list nodes so common error paths can
safely detach them. ENETC also inspects its callback list outside the
setup callback, so protect those accesses with the same mutex.
This fixes the following netdevsim failure:
list_del corruption. next->prev should be ffff88800b3e30c0,
but was ffff88800b539e40.
WARNING: CPU: 0 PID: 79 at lib/list_debug.c:65
RIP: __list_del_entry_valid_or_report
flow_block_cb_setup_simple
nft_block_offload_cmd
nft_flow_rule_offload_commit
nf_tables_commit
nfnetlink_rcv_batch
Fixes: c9626a2cbdb2 ("netfilter: nf_tables: add hardware offload support")
Reported-by: Xiang Mei <xmei5@asu.edu>
Link: https://lore.kernel.org/netdev/20260719175028.988301-2-bestswngs@gmail.com/
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
v2:
- Move serialization to common flow-offload paths, including rollback and indirect cleanup.
- Preserve existing callbacks when an indirect bind fails.
v1: https://lore.kernel.org/netdev/20260719175028.988301-2-bestswngs@gmail.com/
review: https://lore.kernel.org/netdev/20260724142321.GL418547@horms.kernel.org/
.../net/ethernet/freescale/enetc/enetc_qos.c | 27 ++++--
include/net/flow_offload.h | 6 +-
net/core/flow_offload.c | 77 +++++++++++++++--
net/netfilter/nf_flow_table_offload.c | 28 +++---
net/netfilter/nf_tables_offload.c | 18 +++-
net/sched/act_api.c | 4 +-
net/sched/cls_api.c | 86 ++++++++++++++++---
7 files changed, 201 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index 7b17bca24f26..c994f878cacd 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -1601,30 +1601,41 @@ int enetc_set_psfp(struct net_device *ndev, bool en)
int enetc_psfp_init(struct enetc_ndev_priv *priv)
{
+ int err = 0;
+
+ flow_block_cb_lock();
if (epsfp.psfp_sfi_bitmap)
- return 0;
+ goto out;
epsfp.psfp_sfi_bitmap = bitmap_zalloc(priv->psfp_cap.max_psfp_filter,
GFP_KERNEL);
- if (!epsfp.psfp_sfi_bitmap)
- return -ENOMEM;
+ if (!epsfp.psfp_sfi_bitmap) {
+ err = -ENOMEM;
+ goto out;
+ }
spin_lock_init(&epsfp.psfp_lock);
if (list_empty(&enetc_block_cb_list))
epsfp.dev_bitmap = 0;
- return 0;
+out:
+ flow_block_cb_unlock();
+ return err;
}
int enetc_psfp_clean(struct enetc_ndev_priv *priv)
{
- if (!list_empty(&enetc_block_cb_list))
- return -EBUSY;
+ int err = -EBUSY;
- clean_psfp_all();
+ flow_block_cb_lock();
+ if (list_empty(&enetc_block_cb_list)) {
+ clean_psfp_all();
+ err = 0;
+ }
+ flow_block_cb_unlock();
- return 0;
+ return err;
}
int enetc_setup_tc_psfp(struct net_device *ndev, void *type_data)
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 70a02ee14308..96a46ba2769d 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -647,6 +647,8 @@ struct flow_block_cb *flow_block_cb_lookup(struct flow_block *block,
void *flow_block_cb_priv(struct flow_block_cb *block_cb);
void flow_block_cb_incref(struct flow_block_cb *block_cb);
unsigned int flow_block_cb_decref(struct flow_block_cb *block_cb);
+void flow_block_cb_lock(void);
+void flow_block_cb_unlock(void);
static inline void flow_block_cb_add(struct flow_block_cb *block_cb,
struct flow_block_offload *offload)
@@ -741,7 +743,9 @@ void flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
int flow_indr_dev_setup_offload(struct net_device *dev, struct Qdisc *sch,
enum tc_setup_type type, void *data,
struct flow_block_offload *bo,
- void (*cleanup)(struct flow_block_cb *block_cb));
+ void (*cleanup)(struct flow_block_cb *block_cb),
+ bool *indr_dev_added);
+void flow_indr_dev_setup_abort(void *data);
bool flow_indr_dev_exists(void);
#endif /* _NET_FLOW_OFFLOAD_H */
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 5071d7fe6ce2..0b4d2a2cbaf8 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -270,6 +270,8 @@ struct flow_block_cb *flow_block_cb_alloc(flow_setup_cb_t *cb,
block_cb->cb_ident = cb_ident;
block_cb->cb_priv = cb_priv;
block_cb->release = release;
+ INIT_LIST_HEAD(&block_cb->driver_list);
+ INIT_LIST_HEAD(&block_cb->indr.list);
return block_cb;
}
@@ -317,11 +319,27 @@ unsigned int flow_block_cb_decref(struct flow_block_cb *block_cb)
}
EXPORT_SYMBOL(flow_block_cb_decref);
+static DEFINE_MUTEX(flow_block_cb_mutex);
+
+void flow_block_cb_lock(void)
+{
+ mutex_lock(&flow_block_cb_mutex);
+}
+EXPORT_SYMBOL(flow_block_cb_lock);
+
+void flow_block_cb_unlock(void)
+{
+ mutex_unlock(&flow_block_cb_mutex);
+}
+EXPORT_SYMBOL(flow_block_cb_unlock);
+
bool flow_block_cb_is_busy(flow_setup_cb_t *cb, void *cb_ident,
struct list_head *driver_block_list)
{
struct flow_block_cb *block_cb;
+ lockdep_assert_held(&flow_block_cb_mutex);
+
list_for_each_entry(block_cb, driver_block_list, driver_list) {
if (block_cb->cb == cb &&
block_cb->cb_ident == cb_ident)
@@ -340,6 +358,8 @@ int flow_block_cb_setup_simple(struct flow_block_offload *f,
{
struct flow_block_cb *block_cb;
+ lockdep_assert_held(&flow_block_cb_mutex);
+
if (ingress_only &&
f->binder_type != FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
return -EOPNOTSUPP;
@@ -431,12 +451,14 @@ int flow_indr_dev_register(flow_indr_block_bind_cb_t *cb, void *cb_priv)
{
struct flow_indr_dev *indr_dev;
+ flow_block_cb_lock();
mutex_lock(&flow_indr_block_lock);
list_for_each_entry(indr_dev, &flow_block_indr_dev_list, list) {
if (indr_dev->cb == cb &&
indr_dev->cb_priv == cb_priv) {
refcount_inc(&indr_dev->refcnt);
mutex_unlock(&flow_indr_block_lock);
+ flow_block_cb_unlock();
return 0;
}
}
@@ -444,12 +466,14 @@ int flow_indr_dev_register(flow_indr_block_bind_cb_t *cb, void *cb_priv)
indr_dev = flow_indr_dev_alloc(cb, cb_priv);
if (!indr_dev) {
mutex_unlock(&flow_indr_block_lock);
+ flow_block_cb_unlock();
return -ENOMEM;
}
list_add(&indr_dev->list, &flow_block_indr_dev_list);
existing_qdiscs_register(cb, cb_priv);
mutex_unlock(&flow_indr_block_lock);
+ flow_block_cb_unlock();
tcf_action_reoffload_cb(cb, cb_priv, true);
@@ -465,8 +489,10 @@ static void __flow_block_indr_cleanup(void (*release)(void *cb_priv),
list_for_each_entry_safe(this, next, &flow_block_indr_list, indr.list) {
if (this->release == release &&
- this->indr.cb_priv == cb_priv)
+ this->indr.cb_priv == cb_priv) {
+ list_del_init(&this->driver_list);
list_move(&this->indr.list, cleanup_list);
+ }
}
}
@@ -486,6 +512,7 @@ void flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
struct flow_indr_dev *this, *next, *indr_dev = NULL;
LIST_HEAD(cleanup_list);
+ flow_block_cb_lock();
mutex_lock(&flow_indr_block_lock);
list_for_each_entry_safe(this, next, &flow_block_indr_dev_list, list) {
if (this->cb == cb &&
@@ -499,11 +526,13 @@ void flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
if (!indr_dev) {
mutex_unlock(&flow_indr_block_lock);
+ flow_block_cb_unlock();
return;
}
__flow_block_indr_cleanup(release, cb_priv, &cleanup_list);
mutex_unlock(&flow_indr_block_lock);
+ flow_block_cb_unlock();
tcf_action_reoffload_cb(cb, cb_priv, false);
flow_block_indr_notify(&cleanup_list);
@@ -603,18 +632,31 @@ static int indir_dev_remove(void *data)
int flow_indr_dev_setup_offload(struct net_device *dev, struct Qdisc *sch,
enum tc_setup_type type, void *data,
struct flow_block_offload *bo,
- void (*cleanup)(struct flow_block_cb *block_cb))
+ void (*cleanup)(struct flow_block_cb *block_cb),
+ bool *indr_dev_added)
{
struct flow_indr_dev *this;
u32 count = 0;
int err;
+ lockdep_assert_held(&flow_block_cb_mutex);
+
+ if (indr_dev_added)
+ *indr_dev_added = false;
+
mutex_lock(&flow_indr_block_lock);
if (bo) {
- if (bo->command == FLOW_BLOCK_BIND)
- indir_dev_add(data, dev, sch, type, cleanup, bo);
- else if (bo->command == FLOW_BLOCK_UNBIND)
+ if (bo->command == FLOW_BLOCK_BIND) {
+ err = indir_dev_add(data, dev, sch, type, cleanup, bo);
+ if (!err) {
+ if (indr_dev_added)
+ *indr_dev_added = true;
+ } else if (err != -EEXIST) {
+ goto out_unlock;
+ }
+ } else if (bo->command == FLOW_BLOCK_UNBIND) {
indir_dev_remove(data);
+ }
}
list_for_each_entry(this, &flow_block_indr_dev_list, list) {
@@ -623,14 +665,35 @@ int flow_indr_dev_setup_offload(struct net_device *dev, struct Qdisc *sch,
count++;
}
+ err = (bo && list_empty(&bo->cb_list)) ? -EOPNOTSUPP : count;
+
+out_unlock:
mutex_unlock(&flow_indr_block_lock);
- return (bo && list_empty(&bo->cb_list)) ? -EOPNOTSUPP : count;
+ return err;
}
EXPORT_SYMBOL(flow_indr_dev_setup_offload);
+void flow_indr_dev_setup_abort(void *data)
+{
+ lockdep_assert_held(&flow_block_cb_mutex);
+
+ mutex_lock(&flow_indr_block_lock);
+ WARN_ON_ONCE(indir_dev_remove(data));
+ mutex_unlock(&flow_indr_block_lock);
+}
+EXPORT_SYMBOL(flow_indr_dev_setup_abort);
+
bool flow_indr_dev_exists(void)
{
- return !list_empty(&flow_block_indr_dev_list);
+ bool exists;
+
+ flow_block_cb_lock();
+ mutex_lock(&flow_indr_block_lock);
+ exists = !list_empty(&flow_block_indr_dev_list);
+ mutex_unlock(&flow_indr_block_lock);
+ flow_block_cb_unlock();
+
+ return exists;
}
EXPORT_SYMBOL(flow_indr_dev_exists);
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index 801a3dd9ceea..2612978b27b4 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -1186,7 +1186,6 @@ static int nf_flow_table_block_setup(struct nf_flowtable *flowtable,
struct flow_block_cb *block_cb, *next;
int err = 0;
- down_write(&flowtable->flow_block_lock);
switch (cmd) {
case FLOW_BLOCK_BIND:
list_splice(&bo->cb_list, &flowtable->flow_block.cb_list);
@@ -1201,8 +1200,6 @@ static int nf_flow_table_block_setup(struct nf_flowtable *flowtable,
WARN_ON_ONCE(1);
err = -EOPNOTSUPP;
}
- up_write(&flowtable->flow_block_lock);
-
return err;
}
@@ -1230,7 +1227,6 @@ static void nf_flow_table_indr_cleanup(struct flow_block_cb *block_cb)
nf_flow_table_gc_cleanup(flowtable, dev);
down_write(&flowtable->flow_block_lock);
list_del(&block_cb->list);
- list_del(&block_cb->driver_list);
flow_block_cb_free(block_cb);
up_write(&flowtable->flow_block_lock);
}
@@ -1239,13 +1235,15 @@ static int nf_flow_table_indr_offload_cmd(struct flow_block_offload *bo,
struct nf_flowtable *flowtable,
struct net_device *dev,
enum flow_block_command cmd,
- struct netlink_ext_ack *extack)
+ struct netlink_ext_ack *extack,
+ bool *indr_dev_added)
{
nf_flow_table_block_offload_init(bo, dev_net(dev), cmd, flowtable,
extack);
return flow_indr_dev_setup_offload(dev, NULL, TC_SETUP_FT, flowtable, bo,
- nf_flow_table_indr_cleanup);
+ nf_flow_table_indr_cleanup,
+ indr_dev_added);
}
static int nf_flow_table_offload_cmd(struct flow_block_offload *bo,
@@ -1258,9 +1256,7 @@ static int nf_flow_table_offload_cmd(struct flow_block_offload *bo,
nf_flow_table_block_offload_init(bo, dev_net(dev), cmd, flowtable,
extack);
- down_write(&flowtable->flow_block_lock);
err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_FT, bo);
- up_write(&flowtable->flow_block_lock);
if (err < 0)
return err;
@@ -1273,21 +1269,29 @@ int nf_flow_table_offload_setup(struct nf_flowtable *flowtable,
{
struct netlink_ext_ack extack = {};
struct flow_block_offload bo;
+ bool indr_dev_added = false;
int err;
if (!nf_flowtable_hw_offload(flowtable))
return nf_flow_offload_xdp_setup(flowtable, dev, cmd);
+ flow_block_cb_lock();
+ down_write(&flowtable->flow_block_lock);
if (dev->netdev_ops->ndo_setup_tc)
err = nf_flow_table_offload_cmd(&bo, flowtable, dev, cmd,
&extack);
else
err = nf_flow_table_indr_offload_cmd(&bo, flowtable, dev, cmd,
- &extack);
- if (err < 0)
- return err;
+ &extack,
+ &indr_dev_added);
+ if (err >= 0)
+ err = nf_flow_table_block_setup(flowtable, &bo, cmd);
+ if (err < 0 && indr_dev_added)
+ flow_indr_dev_setup_abort(flowtable);
+ up_write(&flowtable->flow_block_lock);
+ flow_block_cb_unlock();
- return nf_flow_table_block_setup(flowtable, &bo, cmd);
+ return err;
}
EXPORT_SYMBOL_GPL(nf_flow_table_offload_setup);
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 8998a24651ff..7189e8c4e5a0 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -414,7 +414,6 @@ static void nft_indr_block_cleanup(struct flow_block_cb *block_cb)
basechain, &extack);
nft_net = nft_pernet(net);
mutex_lock(&nft_net->commit_mutex);
- list_del(&block_cb->driver_list);
list_move(&block_cb->list, &bo.cb_list);
nft_flow_offload_unbind(&bo, basechain);
mutex_unlock(&nft_net->commit_mutex);
@@ -426,19 +425,28 @@ static int nft_indr_block_offload_cmd(struct nft_base_chain *basechain,
{
struct netlink_ext_ack extack = {};
struct flow_block_offload bo;
+ bool indr_dev_added = false;
int err;
nft_flow_block_offload_init(&bo, dev_net(dev), cmd, basechain, &extack);
err = flow_indr_dev_setup_offload(dev, NULL, TC_SETUP_BLOCK, basechain, &bo,
- nft_indr_block_cleanup);
- if (err < 0)
+ nft_indr_block_cleanup,
+ &indr_dev_added);
+ if (err < 0) {
+ if (indr_dev_added)
+ flow_indr_dev_setup_abort(basechain);
return err;
+ }
if (list_empty(&bo.cb_list))
return -EOPNOTSUPP;
- return nft_block_setup(basechain, &bo, cmd);
+ err = nft_block_setup(basechain, &bo, cmd);
+ if (err < 0 && indr_dev_added)
+ flow_indr_dev_setup_abort(basechain);
+
+ return err;
}
static int nft_chain_offload_cmd(struct nft_base_chain *basechain,
@@ -447,10 +455,12 @@ static int nft_chain_offload_cmd(struct nft_base_chain *basechain,
{
int err;
+ flow_block_cb_lock();
if (dev->netdev_ops->ndo_setup_tc)
err = nft_block_offload_cmd(basechain, dev, cmd);
else
err = nft_indr_block_offload_cmd(basechain, dev, cmd);
+ flow_block_cb_unlock();
return err;
}
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f141634df214..317b026f0463 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -205,8 +205,10 @@ static int tcf_action_offload_cmd_ex(struct flow_offload_action *fl_act,
{
int err;
+ flow_block_cb_lock();
err = flow_indr_dev_setup_offload(NULL, NULL, TC_SETUP_ACT,
- fl_act, NULL, NULL);
+ fl_act, NULL, NULL, NULL);
+ flow_block_cb_unlock();
if (err < 0)
return err;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index fee4524adc98..1c230e33e1a5 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -808,7 +808,6 @@ static void tc_block_indr_cleanup(struct flow_block_cb *block_cb)
&extack);
rtnl_lock();
down_write(&block->cb_lock);
- list_del(&block_cb->driver_list);
list_move(&block_cb->list, &bo.cb_list);
tcf_block_unbind(block, &bo);
up_write(&block->cb_lock);
@@ -824,17 +823,20 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
struct net_device *dev, struct Qdisc *sch,
struct tcf_block_ext_info *ei,
enum flow_block_command command,
- struct netlink_ext_ack *extack)
+ struct netlink_ext_ack *extack,
+ bool *indr_dev_added,
+ unsigned int *indr_cb_count,
+ bool *indr_unlocked_driver_cb)
{
struct flow_block_offload bo = {};
+ unsigned int cb_count;
+ int err;
tcf_block_offload_init(&bo, dev, sch, command, ei->binder_type,
&block->flow_block, tcf_block_shared(block),
extack);
if (dev->netdev_ops->ndo_setup_tc) {
- int err;
-
err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_BLOCK, &bo);
if (err < 0) {
if (err != -EOPNOTSUPP)
@@ -845,21 +847,66 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
return tcf_block_setup(block, &bo);
}
- flow_indr_dev_setup_offload(dev, sch, TC_SETUP_BLOCK, block, &bo,
- tc_block_indr_cleanup);
- tcf_block_setup(block, &bo);
+ err = flow_indr_dev_setup_offload(dev, sch, TC_SETUP_BLOCK, block, &bo,
+ tc_block_indr_cleanup,
+ indr_dev_added);
+ if (err < 0)
+ return err;
+ cb_count = list_count_nodes(&bo.cb_list);
+ err = tcf_block_setup(block, &bo);
+ if (err)
+ return err;
+ if (indr_cb_count)
+ *indr_cb_count = cb_count;
+ if (indr_unlocked_driver_cb)
+ *indr_unlocked_driver_cb = bo.unlocked_driver_cb;
return -EOPNOTSUPP;
}
+static void
+tcf_block_indr_bind_rollback(struct tcf_block *block, struct net_device *dev,
+ struct Qdisc *sch, struct tcf_block_ext_info *ei,
+ unsigned int cb_count, bool unlocked_driver_cb,
+ bool remove_indr_dev)
+{
+ struct flow_block_offload bo = {};
+ struct flow_block_cb *block_cb;
+
+ tcf_block_offload_init(&bo, dev, sch, FLOW_BLOCK_UNBIND,
+ ei->binder_type, &block->flow_block,
+ tcf_block_shared(block), NULL);
+ bo.unlocked_driver_cb = unlocked_driver_cb;
+
+ if (remove_indr_dev)
+ flow_indr_dev_setup_abort(block);
+
+ /* tcf_block_bind() splices this bind's callbacks at the list head. */
+ while (cb_count--) {
+ if (WARN_ON_ONCE(list_empty(&block->flow_block.cb_list)))
+ break;
+
+ block_cb = list_first_entry(&block->flow_block.cb_list,
+ struct flow_block_cb, list);
+ list_del_init(&block_cb->driver_list);
+ list_del_init(&block_cb->indr.list);
+ list_move(&block_cb->list, &bo.cb_list);
+ }
+ tcf_block_unbind(block, &bo);
+}
+
static int tcf_block_offload_bind(struct tcf_block *block, struct Qdisc *q,
struct tcf_block_ext_info *ei,
struct netlink_ext_ack *extack)
{
struct net_device *dev = q->dev_queue->dev;
+ bool indr_unlocked_driver_cb = false;
+ bool indr_dev_added = false;
+ unsigned int indr_cb_count = 0;
int err;
down_write(&block->cb_lock);
+ flow_block_cb_lock();
/* If tc offload feature is disabled and the block we try to bind
* to already has some offloaded filters, forbid to bind.
@@ -872,14 +919,15 @@ static int tcf_block_offload_bind(struct tcf_block *block, struct Qdisc *q,
goto err_unlock;
}
- err = tcf_block_offload_cmd(block, dev, q, ei, FLOW_BLOCK_BIND, extack);
+ err = tcf_block_offload_cmd(block, dev, q, ei, FLOW_BLOCK_BIND, extack,
+ &indr_dev_added, &indr_cb_count,
+ &indr_unlocked_driver_cb);
if (err == -EOPNOTSUPP)
goto no_offload_dev_inc;
if (err)
goto err_unlock;
- up_write(&block->cb_lock);
- return 0;
+ goto out_unlock;
no_offload_dev_inc:
if (tcf_block_offload_in_use(block))
@@ -888,6 +936,15 @@ static int tcf_block_offload_bind(struct tcf_block *block, struct Qdisc *q,
err = 0;
block->nooffloaddevcnt++;
err_unlock:
+ if (err && indr_cb_count)
+ tcf_block_indr_bind_rollback(block, dev, q, ei,
+ indr_cb_count,
+ indr_unlocked_driver_cb,
+ indr_dev_added);
+ else if (err && indr_dev_added)
+ flow_indr_dev_setup_abort(block);
+out_unlock:
+ flow_block_cb_unlock();
up_write(&block->cb_lock);
return err;
}
@@ -899,14 +956,18 @@ static void tcf_block_offload_unbind(struct tcf_block *block, struct Qdisc *q,
int err;
down_write(&block->cb_lock);
- err = tcf_block_offload_cmd(block, dev, q, ei, FLOW_BLOCK_UNBIND, NULL);
+ flow_block_cb_lock();
+ err = tcf_block_offload_cmd(block, dev, q, ei, FLOW_BLOCK_UNBIND, NULL,
+ NULL, NULL, NULL);
if (err == -EOPNOTSUPP)
goto no_offload_dev_dec;
+ flow_block_cb_unlock();
up_write(&block->cb_lock);
return;
no_offload_dev_dec:
WARN_ON(block->nooffloaddevcnt-- == 0);
+ flow_block_cb_unlock();
up_write(&block->cb_lock);
}
@@ -1649,7 +1710,8 @@ static int tcf_block_bind(struct tcf_block *block,
err_unroll:
list_for_each_entry_safe(block_cb, next, &bo->cb_list, list) {
- list_del(&block_cb->driver_list);
+ list_del_init(&block_cb->driver_list);
+ list_del_init(&block_cb->indr.list);
if (i-- > 0) {
list_del(&block_cb->list);
tcf_block_playback_offloads(block, block_cb->cb,
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-26 12:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-19 17:50 [PATCH net] netdevsim: tc: serialize access to nsim_block_cb_list Weiming Shi
2026-07-24 14:23 ` Simon Horman
2026-07-26 8:26 ` Weiming Shi
2026-07-26 12:03 ` [PATCH net v2] net: flow_offload: serialize driver callback lists Weiming Shi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox