The bridge-stp usermode helper is currently restricted to the initial
network namespace, preventing userspace STP daemons (e.g. mstpd) from
operating on bridges in other network namespaces. Since commit
ff62198553e4 ("bridge: Only call /sbin/bridge-stp for the initial
network namespace"), bridges in non-init namespaces silently fall back
to kernel STP with no way to use userspace STP.

Add a new bridge attribute IFLA_BR_STP_MODE that allows explicit
per-bridge control over STP mode selection:

  BR_STP_MODE_AUTO (default) - Existing behavior: invoke the
    /sbin/bridge-stp helper in init_net only; fall back to kernel STP
    if it fails or in non-init namespaces.

  BR_STP_MODE_USER - Directly enable userspace STP (BR_USER_STP)
    without invoking the helper. Works in any network namespace. The
    caller is responsible for registering the bridge with the STP
    daemon after enabling STP.

  BR_STP_MODE_KERNEL - Directly enable kernel STP (BR_KERNEL_STP)
    without invoking the helper.

The mode can only be changed while STP is disabled (-EBUSY otherwise).
IFLA_BR_STP_MODE is processed before IFLA_BR_STP_STATE in
br_changelink(), so both can be set atomically in a single netlink
message.

This eliminates the need for call_usermodehelper() in user/kernel
modes, addressing the security concerns discussed in the thread at
https://lore.kernel.org/netdev/[email protected]/ and providing
a cleaner alternative to extending the helper into namespaces.

Suggested-by: Ido Schimmel <[email protected]>
Reviewed-by: Ido Schimmel <[email protected]>
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Andy Roulin <[email protected]>
---
 include/uapi/linux/if_link.h | 40 ++++++++++++++++++++++++++++++++++++
 net/bridge/br_device.c       |  1 +
 net/bridge/br_netlink.c      | 18 +++++++++++++++-
 net/bridge/br_private.h      |  1 +
 net/bridge/br_stp_if.c       | 17 ++++++++-------
 5 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 83a96c56b8cad..87b2b671ec182 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -744,6 +744,11 @@ enum in6_addr_gen_mode {
  * @IFLA_BR_FDB_MAX_LEARNED
  *   Set the number of max dynamically learned FDB entries for the current
  *   bridge.
+ *
+ * @IFLA_BR_STP_MODE
+ *   Set the STP mode for the bridge, which controls how the bridge
+ *   selects between userspace and kernel STP. The valid values are
+ *   documented below in the ``BR_STP_MODE_*`` constants.
  */
 enum {
        IFLA_BR_UNSPEC,
@@ -796,11 +801,46 @@ enum {
        IFLA_BR_MCAST_QUERIER_STATE,
        IFLA_BR_FDB_N_LEARNED,
        IFLA_BR_FDB_MAX_LEARNED,
+       IFLA_BR_STP_MODE,
        __IFLA_BR_MAX,
 };
 
 #define IFLA_BR_MAX    (__IFLA_BR_MAX - 1)
 
+/**
+ * DOC: Bridge STP mode values
+ *
+ * @BR_STP_MODE_AUTO
+ *   Default. The kernel invokes the ``/sbin/bridge-stp`` helper to hand
+ *   the bridge to a userspace STP daemon (e.g. mstpd). Only attempted in
+ *   the initial network namespace; in other namespaces this falls back to
+ *   kernel STP.
+ *
+ * @BR_STP_MODE_USER
+ *   Directly enable userspace STP (``BR_USER_STP``) without invoking the
+ *   ``/sbin/bridge-stp`` helper. Works in any network namespace. The
+ *   caller is responsible for registering the bridge with the userspace
+ *   STP daemon after enabling STP, and for deregistering it before
+ *   disabling STP.
+ *
+ * @BR_STP_MODE_KERNEL
+ *   Directly enable kernel STP (``BR_KERNEL_STP``) without invoking the
+ *   helper.
+ *
+ * The mode controls how the bridge selects between userspace and kernel
+ * STP when STP is enabled via ``IFLA_BR_STP_STATE``. It can only be
+ * changed while STP is disabled (``IFLA_BR_STP_STATE`` == 0), returns
+ * ``-EBUSY`` otherwise. The default value is ``BR_STP_MODE_AUTO``.
+ */
+enum {
+       BR_STP_MODE_AUTO,
+       BR_STP_MODE_USER,
+       BR_STP_MODE_KERNEL,
+       __BR_STP_MODE_MAX
+};
+
+#define BR_STP_MODE_MAX (__BR_STP_MODE_MAX - 1)
+
 struct ifla_bridge_id {
        __u8    prio[2];
        __u8    addr[6]; /* ETH_ALEN */
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index f7502e62dd357..a35ceae0a6f2c 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -518,6 +518,7 @@ void br_dev_setup(struct net_device *dev)
        ether_addr_copy(br->group_addr, eth_stp_addr);
 
        br->stp_enabled = BR_NO_STP;
+       br->stp_mode = BR_STP_MODE_AUTO;
        br->group_fwd_mask = BR_GROUPFWD_DEFAULT;
        br->group_fwd_mask_required = BR_GROUPFWD_DEFAULT;
 
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 0264730938f4b..4c607d5d17a49 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1270,6 +1270,9 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] 
= {
                NLA_POLICY_EXACT_LEN(sizeof(struct br_boolopt_multi)),
        [IFLA_BR_FDB_N_LEARNED] = { .type = NLA_REJECT },
        [IFLA_BR_FDB_MAX_LEARNED] = { .type = NLA_U32 },
+       [IFLA_BR_STP_MODE] = NLA_POLICY_RANGE(NLA_U32,
+                                             BR_STP_MODE_AUTO,
+                                             BR_STP_MODE_MAX),
 };
 
 static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
@@ -1306,6 +1309,17 @@ static int br_changelink(struct net_device *brdev, 
struct nlattr *tb[],
                        return err;
        }
 
+       if (data[IFLA_BR_STP_MODE]) {
+               u32 mode = nla_get_u32(data[IFLA_BR_STP_MODE]);
+
+               if (br->stp_enabled != BR_NO_STP) {
+                       NL_SET_ERR_MSG_MOD(extack,
+                                          "Can't change STP mode while STP is 
enabled");
+                       return -EBUSY;
+               }
+               br->stp_mode = mode;
+       }
+
        if (data[IFLA_BR_STP_STATE]) {
                u32 stp_enabled = nla_get_u32(data[IFLA_BR_STP_STATE]);
 
@@ -1634,6 +1648,7 @@ static size_t br_get_size(const struct net_device *brdev)
               nla_total_size(sizeof(u8)) +     /* IFLA_BR_NF_CALL_ARPTABLES */
 #endif
               nla_total_size(sizeof(struct br_boolopt_multi)) + /* 
IFLA_BR_MULTI_BOOLOPT */
+              nla_total_size(sizeof(u32)) +    /* IFLA_BR_STP_MODE */
               0;
 }
 
@@ -1686,7 +1701,8 @@ static int br_fill_info(struct sk_buff *skb, const struct 
net_device *brdev)
            nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm) ||
            nla_put_u32(skb, IFLA_BR_FDB_N_LEARNED,
                        atomic_read(&br->fdb_n_learned)) ||
-           nla_put_u32(skb, IFLA_BR_FDB_MAX_LEARNED, br->fdb_max_learned))
+           nla_put_u32(skb, IFLA_BR_FDB_MAX_LEARNED, br->fdb_max_learned) ||
+           nla_put_u32(skb, IFLA_BR_STP_MODE, br->stp_mode))
                return -EMSGSIZE;
 
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6dbca845e625d..e4bb9c3f28726 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -540,6 +540,7 @@ struct net_bridge {
                BR_KERNEL_STP,          /* old STP in kernel */
                BR_USER_STP,            /* new RSTP in userspace */
        } stp_enabled;
+       u32                             stp_mode;
 
        struct net_bridge_mcast         multicast_ctx;
 
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index cc4b27ff1b088..fa2271c5d84fe 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -149,7 +149,9 @@ static void br_stp_start(struct net_bridge *br)
 {
        int err = -ENOENT;
 
-       if (net_eq(dev_net(br->dev), &init_net))
+       /* AUTO mode: try bridge-stp helper in init_net only */
+       if (br->stp_mode == BR_STP_MODE_AUTO &&
+           net_eq(dev_net(br->dev), &init_net))
                err = br_stp_call_user(br, "start");
 
        if (err && err != -ENOENT)
@@ -162,7 +164,7 @@ static void br_stp_start(struct net_bridge *br)
        else if (br->bridge_forward_delay > BR_MAX_FORWARD_DELAY)
                __br_set_forward_delay(br, BR_MAX_FORWARD_DELAY);
 
-       if (!err) {
+       if (br->stp_mode == BR_STP_MODE_USER || !err) {
                br->stp_enabled = BR_USER_STP;
                br_debug(br, "userspace STP started\n");
        } else {
@@ -180,12 +182,13 @@ static void br_stp_start(struct net_bridge *br)
 
 static void br_stp_stop(struct net_bridge *br)
 {
-       int err;
-
        if (br->stp_enabled == BR_USER_STP) {
-               err = br_stp_call_user(br, "stop");
-               if (err)
-                       br_err(br, "failed to stop userspace STP (%d)\n", err);
+               if (br->stp_mode == BR_STP_MODE_AUTO) {
+                       int err = br_stp_call_user(br, "stop");
+
+                       if (err)
+                               br_err(br, "failed to stop userspace STP 
(%d)\n", err);
+               }
 
                /* To start timers on any ports left in blocking */
                spin_lock_bh(&br->lock);
-- 
2.43.0


Reply via email to