[PATCH net] net: bridge: dynamically allocate frame_type instances for CFM and MRP

[PATCH net] net: bridge: dynamically allocate frame_type instances for CFM and MRP

[Eric Dumazet]

struct br_frame_type contains an embedded struct hlist_node list, which
is linked into a bridge's per-instance br->frame_type_list via
br_add_frame(br, ft).

Both cfm_frame_type and mrp_frame_type were declared as static global
variables. When multiple bridge devices enable CFM MEPs or MRP instances
concurrently (e.g. across different network namespaces), they share
the same static global br_frame_type node.

Calling br_add_frame() on a second bridge links the same hlist_node into
the second bridge's frame_type_list, corrupting the first bridge's list.
Later, when br_del_frame() deletes the node from one bridge and poisons
its list pointers, a subsequent teardown on another bridge walks its
frame_type_list and dereferences the poisoned pointer in hlist_del_rcu(),
triggering a general protection fault.

Furthermore, sharing or embedding fixed br_frame_type instances leads to
RCU node reuse violations: if MEPs or MRP instances are repeatedly added
and deleted, hlist_del_rcu() unlinks the node without waiting for an RCU
grace period, and a subsequent addition immediately re-inserts and modifies
the node while concurrent lockless readers in br_handle_frame() /
br_process_frame_type() may still be traversing it.

Fix this by dynamically allocating struct br_frame_type upon registration
in br_add_frame() and freeing it with kfree_rcu() in br_del_frame(). Also
ensure all registered frame types are cleaned up during bridge deletion via
br_del_frame_all().

Fixes: 90c628dd47ff ("net: bridge: extend the process of special frames")
Fixes: dc32cbb3dbd7 ("bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.")
Reported-by: syzbot+1df7473ef265fe8ba6e1@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6a9a137f.9266084e.bf0d7.02e3.GAE@google.com/
Signed-off-by: Eric Dumazet <edumazet@google.com>
--
Cc: Nikolay Aleksandrov <razor@blackwall.org>
Cc: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_cfm.c     | 17 +++++++++--------
 net/bridge/br_if.c      |  2 ++
 net/bridge/br_input.c   | 40 +++++++++++++++++++++++++++++++++++-----
 net/bridge/br_mrp.c     | 16 +++++++---------
 net/bridge/br_private.h |  8 ++++++--
 5 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
index dea56fffa1c19fab589bc5ec799cdea5d35dd791..579024858829886a2f22001a51c1740eb23b27d9 100644
--- a/net/bridge/br_cfm.c
+++ b/net/bridge/br_cfm.c
@@ -489,11 +489,6 @@ static int br_cfm_frame_rx(struct net_bridge_port *port, struct sk_buff *skb)
 	return 1;
 }
 
-static struct br_frame_type cfm_frame_type __read_mostly = {
-	.type = cpu_to_be16(ETH_P_CFM),
-	.frame_handler = br_cfm_frame_rx,
-};
-
 int br_cfm_mep_create(struct net_bridge *br,
 		      const u32 instance,
 		      struct br_cfm_mep_create *const create,
@@ -501,6 +496,7 @@ int br_cfm_mep_create(struct net_bridge *br,
 {
 	struct net_bridge_port *p;
 	struct br_cfm_mep *mep;
+	int err;
 
 	ASSERT_RTNL();
 
@@ -558,8 +554,13 @@ int br_cfm_mep_create(struct net_bridge *br,
 	INIT_HLIST_HEAD(&mep->peer_mep_list);
 	INIT_DELAYED_WORK(&mep->ccm_tx_dwork, ccm_tx_work_expired);
 
-	if (hlist_empty(&br->mep_list))
-		br_add_frame(br, &cfm_frame_type);
+	if (hlist_empty(&br->mep_list)) {
+		err = br_add_frame(br, cpu_to_be16(ETH_P_CFM), br_cfm_frame_rx);
+		if (err) {
+			kfree(mep);
+			return err;
+		}
+	}
 
 	hlist_add_tail_rcu(&mep->head, &br->mep_list);
 
@@ -588,7 +589,7 @@ static void mep_delete_implementation(struct net_bridge *br,
 	kfree_rcu(mep, rcu);
 
 	if (hlist_empty(&br->mep_list))
-		br_del_frame(br, &cfm_frame_type);
+		br_del_frame(br, cpu_to_be16(ETH_P_CFM));
 }
 
 int br_cfm_mep_delete(struct net_bridge *br,
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index c52613431f886cbe832e499195d40f0a16908870..fe8e9a47e1ac106dd611eb96a25a216d0b5bc2d5 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -397,6 +397,8 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
 	timer_shutdown_sync(&br->tcn_timer);
 	cancel_delayed_work_sync(&br->gc_work);
 
+	br_del_frame_all(br);
+
 	br_sysfs_delbr(br->dev);
 	unregister_netdevice_queue(br->dev, head);
 }
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index d87a5f9fa92b70d06d0f47cb884c72baf253ed0e..07efd0fbcf9cd5357a83de22338c3dca94623ee1 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -468,18 +468,48 @@ rx_handler_func_t *br_get_rx_handler(const struct net_device *dev)
 	return br_handle_frame;
 }
 
-void br_add_frame(struct net_bridge *br, struct br_frame_type *ft)
+int br_add_frame(struct net_bridge *br, __be16 type,
+		 int (*frame_handler)(struct net_bridge_port *port,
+				      struct sk_buff *skb))
 {
+	struct br_frame_type *ft;
+
+	hlist_for_each_entry(ft, &br->frame_type_list, list) {
+		if (ft->type == type)
+			return -EEXIST;
+	}
+
+	ft = kmalloc_obj(*ft);
+	if (!ft)
+		return -ENOMEM;
+
+	ft->type = type;
+	ft->frame_handler = frame_handler;
 	hlist_add_head_rcu(&ft->list, &br->frame_type_list);
+
+	return 0;
 }
 
-void br_del_frame(struct net_bridge *br, struct br_frame_type *ft)
+void br_del_frame(struct net_bridge *br, __be16 type)
 {
-	struct br_frame_type *tmp;
+	struct br_frame_type *ft;
 
-	hlist_for_each_entry(tmp, &br->frame_type_list, list)
-		if (ft == tmp) {
+	hlist_for_each_entry(ft, &br->frame_type_list, list) {
+		if (ft->type == type) {
 			hlist_del_rcu(&ft->list);
+			kfree_rcu(ft, rcu);
 			return;
 		}
+	}
+}
+
+void br_del_frame_all(struct net_bridge *br)
+{
+	struct br_frame_type *ft;
+	struct hlist_node *n;
+
+	hlist_for_each_entry_safe(ft, n, &br->frame_type_list, list) {
+		hlist_del_rcu(&ft->list);
+		kfree_rcu(ft, rcu);
+	}
 }
diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
index ef16d07039241a76dde0a0923030024bba2c1c33..511ae8af3d03395beac450f74def5e908feaa4c7 100644
--- a/net/bridge/br_mrp.c
+++ b/net/bridge/br_mrp.c
@@ -8,11 +8,6 @@ static const u8 mrp_in_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x3 };
 
 static int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb);
 
-static struct br_frame_type mrp_frame_type __read_mostly = {
-	.type = cpu_to_be16(ETH_P_MRP),
-	.frame_handler = br_mrp_process,
-};
-
 static bool br_mrp_is_ring_port(struct net_bridge_port *p_port,
 				struct net_bridge_port *s_port,
 				struct net_bridge_port *port)
@@ -486,7 +481,7 @@ static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
 	kfree_rcu(mrp, rcu);
 
 	if (hlist_empty(&br->mrp_list))
-		br_del_frame(br, &mrp_frame_type);
+		br_del_frame(br, cpu_to_be16(ETH_P_MRP));
 }
 
 /* Adds a new MRP instance.
@@ -535,13 +530,16 @@ int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
 	spin_unlock_bh(&br->lock);
 	rcu_assign_pointer(mrp->s_port, p);
 
-	if (hlist_empty(&br->mrp_list))
-		br_add_frame(br, &mrp_frame_type);
-
 	INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
 	INIT_DELAYED_WORK(&mrp->in_test_work, br_mrp_in_test_work_expired);
 	hlist_add_tail_rcu(&mrp->list, &br->mrp_list);
 
+	if (hlist_is_singular_node(&mrp->list, &br->mrp_list)) {
+		err = br_add_frame(br, cpu_to_be16(ETH_P_MRP), br_mrp_process);
+		if (err)
+			goto delete_mrp;
+	}
+
 	err = br_mrp_switchdev_add(br, mrp);
 	if (err)
 		goto delete_mrp;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index d337b1cfb980d0682ceb33ed5d4554b9991b65d4..5163017feb74deed059c592128f6fdb8d2469bf8 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -937,10 +937,14 @@ struct br_frame_type {
 	int			(*frame_handler)(struct net_bridge_port *port,
 						 struct sk_buff *skb);
 	struct hlist_node	list;
+	struct rcu_head		rcu;
 };
 
-void br_add_frame(struct net_bridge *br, struct br_frame_type *ft);
-void br_del_frame(struct net_bridge *br, struct br_frame_type *ft);
+int br_add_frame(struct net_bridge *br, __be16 type,
+		 int (*frame_handler)(struct net_bridge_port *port,
+				      struct sk_buff *skb));
+void br_del_frame(struct net_bridge *br, __be16 type);
+void br_del_frame_all(struct net_bridge *br);
 
 static inline bool br_rx_handler_check_rcu(const struct net_device *dev)
 {
-- 
2.55.0.979.g7e5102b832-goog

Re: [PATCH net] net: bridge: dynamically allocate frame_type instances for CFM and MRP

[Nikolay Aleksandrov]

On 04/09/2026 03:44, Eric Dumazet wrote:
> struct br_frame_type contains an embedded struct hlist_node list, which
> is linked into a bridge's per-instance br->frame_type_list via
> br_add_frame(br, ft).
> 
> Both cfm_frame_type and mrp_frame_type were declared as static global
> variables. When multiple bridge devices enable CFM MEPs or MRP instances
> concurrently (e.g. across different network namespaces), they share
> the same static global br_frame_type node.
> 
> Calling br_add_frame() on a second bridge links the same hlist_node into
> the second bridge's frame_type_list, corrupting the first bridge's list.
> Later, when br_del_frame() deletes the node from one bridge and poisons
> its list pointers, a subsequent teardown on another bridge walks its
> frame_type_list and dereferences the poisoned pointer in hlist_del_rcu(),
> triggering a general protection fault.
> 
> Furthermore, sharing or embedding fixed br_frame_type instances leads to
> RCU node reuse violations: if MEPs or MRP instances are repeatedly added
> and deleted, hlist_del_rcu() unlinks the node without waiting for an RCU
> grace period, and a subsequent addition immediately re-inserts and modifies
> the node while concurrent lockless readers in br_handle_frame() /
> br_process_frame_type() may still be traversing it.
> 
> Fix this by dynamically allocating struct br_frame_type upon registration
> in br_add_frame() and freeing it with kfree_rcu() in br_del_frame(). Also
> ensure all registered frame types are cleaned up during bridge deletion via
> br_del_frame_all().
> 
> Fixes: 90c628dd47ff ("net: bridge: extend the process of special frames")
> Fixes: dc32cbb3dbd7 ("bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.")
> Reported-by: syzbot+1df7473ef265fe8ba6e1@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/6a9a137f.9266084e.bf0d7.02e3.GAE@google.com/
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> --
> Cc: Nikolay Aleksandrov <razor@blackwall.org>
> Cc: Ido Schimmel <idosch@nvidia.com>
> ---
>   net/bridge/br_cfm.c     | 17 +++++++++--------
>   net/bridge/br_if.c      |  2 ++
>   net/bridge/br_input.c   | 40 +++++++++++++++++++++++++++++++++++-----
>   net/bridge/br_mrp.c     | 16 +++++++---------
>   net/bridge/br_private.h |  8 ++++++--
>   5 files changed, 59 insertions(+), 24 deletions(-)
> 

Thanks Eric, but there is already a patch that should take care of this.
A week ago I pinged the previous reporter of these issues and he posted
an updated patch yesterday:
https://lore.kernel.org/netdev/0345b9d5aa60ba416f6738ff1b87140f0a749cb8.1788417901.git.zhilinz@nebusec.ai/

Cheers,
  Nik

Re: [PATCH net] net: bridge: dynamically allocate frame_type instances for CFM and MRP

[Eric Dumazet]

On Fri, Sep 4, 2026 at 8:02 AM Nikolay Aleksandrov <razor@blackwall.org> wrote:
>
> On 04/09/2026 03:44, Eric Dumazet wrote:
> > struct br_frame_type contains an embedded struct hlist_node list, which
> > is linked into a bridge's per-instance br->frame_type_list via
> > br_add_frame(br, ft).
> >
> > Both cfm_frame_type and mrp_frame_type were declared as static global
> > variables. When multiple bridge devices enable CFM MEPs or MRP instances
> > concurrently (e.g. across different network namespaces), they share
> > the same static global br_frame_type node.
> >
> > Calling br_add_frame() on a second bridge links the same hlist_node into
> > the second bridge's frame_type_list, corrupting the first bridge's list.
> > Later, when br_del_frame() deletes the node from one bridge and poisons
> > its list pointers, a subsequent teardown on another bridge walks its
> > frame_type_list and dereferences the poisoned pointer in hlist_del_rcu(),
> > triggering a general protection fault.
> >
> > Furthermore, sharing or embedding fixed br_frame_type instances leads to
> > RCU node reuse violations: if MEPs or MRP instances are repeatedly added
> > and deleted, hlist_del_rcu() unlinks the node without waiting for an RCU
> > grace period, and a subsequent addition immediately re-inserts and modifies
> > the node while concurrent lockless readers in br_handle_frame() /
> > br_process_frame_type() may still be traversing it.
> >
> > Fix this by dynamically allocating struct br_frame_type upon registration
> > in br_add_frame() and freeing it with kfree_rcu() in br_del_frame(). Also
> > ensure all registered frame types are cleaned up during bridge deletion via
> > br_del_frame_all().
> >
> > Fixes: 90c628dd47ff ("net: bridge: extend the process of special frames")
> > Fixes: dc32cbb3dbd7 ("bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.")
> > Reported-by: syzbot+1df7473ef265fe8ba6e1@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/netdev/6a9a137f.9266084e.bf0d7.02e3.GAE@google.com/
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > --
> > Cc: Nikolay Aleksandrov <razor@blackwall.org>
> > Cc: Ido Schimmel <idosch@nvidia.com>
> > ---
> >   net/bridge/br_cfm.c     | 17 +++++++++--------
> >   net/bridge/br_if.c      |  2 ++
> >   net/bridge/br_input.c   | 40 +++++++++++++++++++++++++++++++++++-----
> >   net/bridge/br_mrp.c     | 16 +++++++---------
> >   net/bridge/br_private.h |  8 ++++++--
> >   5 files changed, 59 insertions(+), 24 deletions(-)
> >
>
> Thanks Eric, but there is already a patch that should take care of this.
> A week ago I pinged the previous reporter of these issues and he posted
> an updated patch yesterday:
> https://lore.kernel.org/netdev/0345b9d5aa60ba416f6738ff1b87140f0a749cb8.1788417901.git.zhilinz@nebusec.ai/
>

Oh nice, their patch seems better indeed.

It's interesting that syzbot found the issue around the same time...

Thanks.

pw-bot: rejected