* [PATCH net] hsr: broadcast netlink notifications in the device's net namespace [not found] <CAHPEe=GO=2qqWZPwBB4rrXc3mkD0dznp2K78nCsKwF=c-QwxEw@mail.gmail.com> @ 2026-05-27 7:59 ` Maoyi Xie 2026-05-27 11:11 ` Fernando Fernandez Mancera 0 siblings, 1 reply; 3+ messages in thread From: Maoyi Xie @ 2026-05-27 7:59 UTC (permalink / raw) To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman Cc: Fernando Fernandez Mancera, Jan Vaclav, Andrew Lunn, Taehee Yoo, netdev, linux-kernel, Maoyi Xie, stable The HSR generic netlink family sets .netnsok = true. HSR devices can live in network namespaces other than init_net. Two async notifiers broadcast events with genlmsg_multicast(). They are hsr_nl_ringerror() and hsr_nl_nodedown(). That helper delivers only on the default genl socket in init_net. So the events always land in init_net. The network namespace of the device does not matter. This has two effects. A listener in the device's own namespace never sees its own ring error and node down events. A privileged listener in init_net receives events from HSR devices in other namespaces. The payload carries the peer node MAC (HSR_A_NODE_ADDR) and the slave port ifindex (HSR_A_IFINDEX). It leaks information across network namespaces. Switch both callers to genlmsg_multicast_netns(). Other families with .netnsok = true already do this. Examples are gtp, ovpn, team, batman-adv, netdev-genl, ethtool and handshake. hsr_nl_ringerror() already has the slave port. It uses dev_net(port->dev). hsr_nl_nodedown() takes the namespace from the master port via hsr_port_get_hsr(). Fixes: 09e91dbea0aa ("hsr: set .netnsok flag") Cc: stable@vger.kernel.org Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com> --- This is the fix for the problem I reported on netdev on 2026-05-18 [1]. That thread had no reply, so I am sending the patch and adding the HSR maintainers to Cc. The proof of concept and the test numbers are in that message. [1] https://lore.kernel.org/netdev/CAHPEe=GO=2qqWZPwBB4rrXc3mkD0dznp2K78nCsKwF=c-QwxEw@mail.gmail.com/ net/hsr/hsr_netlink.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c index db0b0af7a692..067ceaf7304b 100644 --- a/net/hsr/hsr_netlink.c +++ b/net/hsr/hsr_netlink.c @@ -247,7 +247,8 @@ void hsr_nl_ringerror(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN], goto nla_put_failure; genlmsg_end(skb, msg_head); - genlmsg_multicast(&hsr_genl_family, skb, 0, 0, GFP_ATOMIC); + genlmsg_multicast_netns(&hsr_genl_family, dev_net(port->dev), + skb, 0, 0, GFP_ATOMIC); return; @@ -283,8 +284,17 @@ void hsr_nl_nodedown(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN]) if (res < 0) goto nla_put_failure; + rcu_read_lock(); + master = hsr_port_get_hsr(hsr, HSR_PT_MASTER); + if (!master) { + rcu_read_unlock(); + goto nla_put_failure; + } + genlmsg_end(skb, msg_head); - genlmsg_multicast(&hsr_genl_family, skb, 0, 0, GFP_ATOMIC); + genlmsg_multicast_netns(&hsr_genl_family, dev_net(master->dev), + skb, 0, 0, GFP_ATOMIC); + rcu_read_unlock(); return; -- 2.34.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] hsr: broadcast netlink notifications in the device's net namespace 2026-05-27 7:59 ` [PATCH net] hsr: broadcast netlink notifications in the device's net namespace Maoyi Xie @ 2026-05-27 11:11 ` Fernando Fernandez Mancera 2026-05-27 23:18 ` Jakub Kicinski 0 siblings, 1 reply; 3+ messages in thread From: Fernando Fernandez Mancera @ 2026-05-27 11:11 UTC (permalink / raw) To: Maoyi Xie, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman Cc: Jan Vaclav, Andrew Lunn, Taehee Yoo, netdev, linux-kernel, stable On 5/27/26 9:59 AM, Maoyi Xie wrote: > The HSR generic netlink family sets .netnsok = true. HSR devices can > live in network namespaces other than init_net. > > Two async notifiers broadcast events with genlmsg_multicast(). They > are hsr_nl_ringerror() and hsr_nl_nodedown(). That helper delivers > only on the default genl socket in init_net. So the events always land > in init_net. The network namespace of the device does not matter. > > This has two effects. A listener in the device's own namespace never > sees its own ring error and node down events. A privileged listener in > init_net receives events from HSR devices in other namespaces. The > payload carries the peer node MAC (HSR_A_NODE_ADDR) and the slave port > ifindex (HSR_A_IFINDEX). It leaks information across network > namespaces. > > Switch both callers to genlmsg_multicast_netns(). Other families with > .netnsok = true already do this. Examples are gtp, ovpn, team, > batman-adv, netdev-genl, ethtool and handshake. > > hsr_nl_ringerror() already has the slave port. It uses > dev_net(port->dev). hsr_nl_nodedown() takes the namespace from the > master port via hsr_port_get_hsr(). > > Fixes: 09e91dbea0aa ("hsr: set .netnsok flag") > Cc: stable@vger.kernel.org > Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com> > --- > This is the fix for the problem I reported on netdev on 2026-05-18 [1]. > That thread had no reply, so I am sending the patch and adding the HSR > maintainers to Cc. The proof of concept and the test numbers are in > that message. > > [1] https://lore.kernel.org/netdev/CAHPEe=GO=2qqWZPwBB4rrXc3mkD0dznp2K78nCsKwF=c-QwxEw@mail.gmail.com/ > > net/hsr/hsr_netlink.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c > index db0b0af7a692..067ceaf7304b 100644 > --- a/net/hsr/hsr_netlink.c > +++ b/net/hsr/hsr_netlink.c > @@ -247,7 +247,8 @@ void hsr_nl_ringerror(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN], > goto nla_put_failure; > > genlmsg_end(skb, msg_head); > - genlmsg_multicast(&hsr_genl_family, skb, 0, 0, GFP_ATOMIC); > + genlmsg_multicast_netns(&hsr_genl_family, dev_net(port->dev), > + skb, 0, 0, GFP_ATOMIC); > > return; > > @@ -283,8 +284,17 @@ void hsr_nl_nodedown(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN]) > if (res < 0) > goto nla_put_failure; > > + rcu_read_lock(); > + master = hsr_port_get_hsr(hsr, HSR_PT_MASTER); > + if (!master) { > + rcu_read_unlock(); > + goto nla_put_failure; > + } > + > genlmsg_end(skb, msg_head); > - genlmsg_multicast(&hsr_genl_family, skb, 0, 0, GFP_ATOMIC); > + genlmsg_multicast_netns(&hsr_genl_family, dev_net(master->dev), > + skb, 0, 0, GFP_ATOMIC); > + rcu_read_unlock(); > The patch makes sense to me. Anyway, I think we can reduce the RCU critical section here. What about moving rcu_read_unlock() after the if and extract the net before unlocking? The benefit is minimal but I think it is worth it. Thanks, Fernando. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] hsr: broadcast netlink notifications in the device's net namespace 2026-05-27 11:11 ` Fernando Fernandez Mancera @ 2026-05-27 23:18 ` Jakub Kicinski 0 siblings, 0 replies; 3+ messages in thread From: Jakub Kicinski @ 2026-05-27 23:18 UTC (permalink / raw) To: Fernando Fernandez Mancera Cc: Maoyi Xie, David S . Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Jan Vaclav, Andrew Lunn, Taehee Yoo, netdev, linux-kernel, stable On Wed, 27 May 2026 13:11:57 +0200 Fernando Fernandez Mancera wrote: > > genlmsg_end(skb, msg_head); > > - genlmsg_multicast(&hsr_genl_family, skb, 0, 0, GFP_ATOMIC); > > + genlmsg_multicast_netns(&hsr_genl_family, dev_net(master->dev), > > + skb, 0, 0, GFP_ATOMIC); > > + rcu_read_unlock(); > > > > The patch makes sense to me. Anyway, I think we can reduce the RCU > critical section here. > > What about moving rcu_read_unlock() after the if and extract the net > before unlocking? The benefit is minimal but I think it is worth it. Not sure TBH, we'd need to take a ref on the netns and allocate a tracker (on DEBUG kernels). One could go either way. I'm replying because I wanted to question whether this is Fixes+stable@ worthy. Sending the notifications to the namespace where the device is makes sense. But it's as much a behavior changes as it is a fix. The commit in question was merged to 5.6, real users clearly don't care. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-27 23:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAHPEe=GO=2qqWZPwBB4rrXc3mkD0dznp2K78nCsKwF=c-QwxEw@mail.gmail.com>
2026-05-27 7:59 ` [PATCH net] hsr: broadcast netlink notifications in the device's net namespace Maoyi Xie
2026-05-27 11:11 ` Fernando Fernandez Mancera
2026-05-27 23:18 ` Jakub Kicinski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox