On 10/05/2026 12:15, Ren Wei wrote:
From: Yilin Zhu <[email protected]>

CFM registers a bridge frame handler when the first MEP is created and
unregisters it when the last MEP is deleted. The registered object also
contains the hlist_node used by the bridge-local frame_type_list.

The CFM frame type is currently global, so enabling CFM on multiple
bridges links the same hlist_node into multiple bridge-local lists. A
later unregister on one bridge can then operate on list state belonging
to another bridge.

Move the CFM frame type into struct net_bridge and register/unregister
the bridge-owned object. This keeps frame handler list membership local
to the bridge while preserving the existing first-MEP/last-MEP lifetime.

Fixes: dc32cbb3dbd7 ("bridge: cfm: Kernel space implementation of CFM. CCM frame RX 
added.")
Cc: [email protected]
Reported-by: Yuan Tan <[email protected]>
Reported-by: Yifan Wu <[email protected]>
Reported-by: Juefei Pu <[email protected]>
Reported-by: Xin Liu <[email protected]>
Co-developed-by: Peihan Liu <[email protected]>
Signed-off-by: Peihan Liu <[email protected]>
Signed-off-by: Yilin Zhu <[email protected]>
Signed-off-by: Ren Wei <[email protected]>
---
  net/bridge/br_cfm.c     | 14 ++++++--------
  net/bridge/br_private.h | 18 +++++++++++-------
  2 files changed, 17 insertions(+), 15 deletions(-)


I think MRP suffers from the same bug, but I also think we can contain the
fix within the packet type structure instead of making the already huge
struct net_bridge even bigger. Also, linking a struct within net_bridge
to a list within the same net_bridge looks weird.

IMO br_add_frame should take a type & a frame_handler, allocate a br_frame_type
dynamically and link it, then br_del_frame should take a type instead of a ptr
and remove that frame type and free it with kfree_rcu. That would require
br_add_frame return value to be checked in the respective cfm/mrp functions.
Warnings for already existing types on add or missing types on del should be
added.

Would you please take care of MRP as well?

Cheers,
 Nik

diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
index 118c7ea48c35..547c7415c0ea 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,
@@ -558,8 +553,11 @@ 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)) {
+               br->cfm_frame_type.type = cpu_to_be16(ETH_P_CFM);
+               br->cfm_frame_type.frame_handler = br_cfm_frame_rx;
+               br_add_frame(br, &br->cfm_frame_type);
+       }
hlist_add_tail_rcu(&mep->head, &br->mep_list); @@ -588,7 +586,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, &br->cfm_frame_type);
  }
int br_cfm_mep_delete(struct net_bridge *br,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index bed1b1d9b282..a3ed9ee826f5 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -61,6 +61,9 @@ typedef struct bridge_id bridge_id;
  typedef struct mac_addr mac_addr;
  typedef __u16 port_id;
+struct net_bridge_port;
+struct sk_buff;
+
  struct bridge_id {
        unsigned char   prio[2];
        unsigned char   addr[ETH_ALEN];
@@ -70,6 +73,13 @@ struct mac_addr {
        unsigned char   addr[ETH_ALEN];
  };
+struct br_frame_type {
+       __be16                  type;
+       int                     (*frame_handler)(struct net_bridge_port *port,
+                                                struct sk_buff *skb);
+       struct hlist_node       list;
+};
+
  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
  /* our own querier */
  struct bridge_mcast_own_query {
@@ -585,6 +595,7 @@ struct net_bridge {
        struct hlist_head               mrp_list;
  #endif
  #if IS_ENABLED(CONFIG_BRIDGE_CFM)
+       struct br_frame_type    cfm_frame_type;
        struct hlist_head               mep_list;
  #endif
  };
@@ -926,13 +937,6 @@ int nbp_backup_change(struct net_bridge_port *p, struct 
net_device *backup_dev);
  int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff 
*skb);
  rx_handler_func_t *br_get_rx_handler(const struct net_device *dev);
-struct br_frame_type {
-       __be16                  type;
-       int                     (*frame_handler)(struct net_bridge_port *port,
-                                                struct sk_buff *skb);
-       struct hlist_node       list;
-};
-
  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);


Reply via email to