(CC'ing bridge maintainers.) Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> writes: > From: Harry Coin <hcoin@xxxxxxxxxxxxxxxxx> > Date: Tue, 11 Jul 2023 16:40:03 -0500 >> On 7/11/23 15:44, Andrew Lunn wrote: >> >>>>>> The current llc_rcv.c around line 166 in net/llc/llc_input.c has >> >>>>>> >> >>>>>> if (!net_eq(dev_net(dev), &init_net)) >> >>>>>> goto drop; >> >>>>>> >> >> Thank you! When you offer your patches, and you hear worries about being >> >> 'invasive', it's worth asking 'compared to what' -- since the 'status quo' >> >> is every bridge with STP in a non default namespace with a loop in it >> >> somewhere will freeze every connected system more solid than ice in >> >> Antarctica. >> > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html >> > >> > say: >> > >> > o It must be obviously correct and tested. >> > o It cannot be bigger than 100 lines, with context. >> > o It must fix only one thing. >> > o It must fix a real bug that bothers people (not a, "This could be a problem..." type thing). >> > >> > git blame shows: >> > >> > commit 721499e8931c5732202481ae24f2dfbf9910f129 >> > Author: YOSHIFUJI Hideaki <yoshfuji@xxxxxxxxxxxxxx> >> > Date: Sat Jul 19 22:34:43 2008 -0700 >> > >> > netns: Use net_eq() to compare net-namespaces for optimization. >> > >> > diff --git a/net/llc/llc_input.c b/net/llc/llc_input.c >> > index 1c45f172991e..57ad974e4d94 100644 >> > --- a/net/llc/llc_input.c >> > +++ b/net/llc/llc_input.c >> > @@ -150,7 +150,7 @@ int llc_rcv(struct sk_buff *skb, struct net_device *dev, >> > int (*rcv)(struct sk_buff *, struct net_device *, >> > struct packet_type *, struct net_device *); >> > >> > - if (dev_net(dev) != &init_net) >> > + if (!net_eq(dev_net(dev), &init_net)) >> > goto drop; >> > >> > /* >> > >> > So this is just an optimization. >> > >> > The test itself was added in >> > >> > ommit e730c15519d09ea528b4d2f1103681fa5937c0e6 >> > Author: Eric W. Biederman <ebiederm@xxxxxxxxxxxx> >> > Date: Mon Sep 17 11:53:39 2007 -0700 >> > >> > [NET]: Make packet reception network namespace safe >> > >> > This patch modifies every packet receive function >> > registered with dev_add_pack() to drop packets if they >> > are not from the initial network namespace. >> > >> > This should ensure that the various network stacks do >> > not receive packets in a anything but the initial network >> > namespace until the code has been converted and is ready >> > for them. >> > >> > Signed-off-by: Eric W. Biederman <ebiederm@xxxxxxxxxxxx> >> > Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx> >> > >> > So that was over 15 years ago. >> > >> > It appears it has not bothered people for over 15 years. >> > >> > Lets wait until we get to see the actual fix. We can then decide how >> > simple/hard it is to back port to stable, if it fulfils the stable >> > rules or not. >> > >> > Andrew >> >> Andrew, fair enough. In the time until it's fixed, the kernel folks >> should publish an advisory and block any attempt to set bridge stp state >> to other than 0 if in a non-default namespace. The alternative is a >> packet flood at whatever the top line speed is should there be a loop >> somewhere in even one connected link. > > Like this ? Someone who didn't notice the issue might complain about > it as regression. > > ---8<--- > diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c > index 75204d36d7f9..a807996ac56b 100644 > --- a/net/bridge/br_stp_if.c > +++ b/net/bridge/br_stp_if.c > @@ -201,6 +201,11 @@ int br_stp_set_enabled(struct net_bridge *br, unsigned long val, > { > ASSERT_RTNL(); > > + if (!net_eq(dev_net(br->dev), &init_net)) { > + NL_SET_ERR_MSG_MOD(extack, "STP can't be enabled in non-root netns"); > + return -EINVAL; > + } > + > if (br_mrp_enabled(br)) { > NL_SET_ERR_MSG_MOD(extack, > "STP can't be enabled if MRP is already enabled"); > ---8<---
