[PATCH net-next v2] hsr: broadcast netlink notifications in the device's net namespace

[PATCH net-next v2] hsr: broadcast netlink notifications in the device's net namespace

[Maoyi Xie]

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).

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().

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
v2: Drop the Fixes and stable tags and target net-next. As discussed,
    this is more of a behavior change than a fix, and the .netnsok
    commit has been in since 5.6 with no report. The multicast stays
    inside the rcu_read_lock, which avoids taking a netns ref on this
    path. Fernando's Reviewed-by carried over.

Inquiry (2026-05-18):
https://lore.kernel.org/netdev/CAHPEe=GO=2qqWZPwBB4rrXc3mkD0dznp2K78nCsKwF=c-QwxEw@mail.gmail.com/
v1 [PATCH net] (2026-05-27):
https://lore.kernel.org/netdev/20260527075924.2707856-1-maoyixie.tju@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 f0ca23da3ab9..eb6d222535d8 100644
--- a/net/hsr/hsr_netlink.c
+++ b/net/hsr/hsr_netlink.c
@@ -252,7 +252,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;
 
@@ -288,8 +289,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

Re: [PATCH net-next v2] hsr: broadcast netlink notifications in the device's net namespace

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
hsr: broadcast netlink notifications in the device's net namespace

This commit modifies the HSR subsystem to broadcast generic netlink
notifications in the device's network namespace instead of init_net. It updates
hsr_nl_ringerror() and hsr_nl_nodedown() to use genlmsg_multicast_netns().

> diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c
> index f0ca23da3ab99..eb6d222535d8b 100644
> --- a/net/hsr/hsr_netlink.c
> +++ b/net/hsr/hsr_netlink.c
[ ... ]
> @@ -288,8 +289,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;

[Severity: High]
Does jumping to nla_put_failure here cause a NULL pointer dereference?

If master is NULL, the code jumps to nla_put_failure, which frees the skb
and then falls through to the fail label:

nla_put_failure:
	kfree_skb(skb);

fail:
	rcu_read_lock();
	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
	netdev_warn(master->dev, "Could not send HSR node down\n");
	rcu_read_unlock();

Since master was not found previously, the re-fetch will likely still be NULL,
meaning the dereference of master->dev in netdev_warn() will crash.

> +	}
> +
>  	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;
>
-- 
pw-bot: cr

Re: [PATCH net-next v2] hsr: broadcast netlink notifications in the device's net namespace

[Fernando Fernandez Mancera]

On 6/2/26 9:06 PM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> hsr: broadcast netlink notifications in the device's net namespace
> 
> This commit modifies the HSR subsystem to broadcast generic netlink
> notifications in the device's network namespace instead of init_net. It updates
> hsr_nl_ringerror() and hsr_nl_nodedown() to use genlmsg_multicast_netns().
> 
>> diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c
>> index f0ca23da3ab99..eb6d222535d8b 100644
>> --- a/net/hsr/hsr_netlink.c
>> +++ b/net/hsr/hsr_netlink.c
> [ ... ]
>> @@ -288,8 +289,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;
> 
> [Severity: High]
> Does jumping to nla_put_failure here cause a NULL pointer dereference?
> 
> If master is NULL, the code jumps to nla_put_failure, which frees the skb
> and then falls through to the fail label:
> 
> nla_put_failure:
> 	kfree_skb(skb);
> 
> fail:
> 	rcu_read_lock();
> 	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
> 	netdev_warn(master->dev, "Could not send HSR node down\n");
> 	rcu_read_unlock();
> 
> Since master was not found previously, the re-fetch will likely still be NULL,
> meaning the dereference of master->dev in netdev_warn() will crash.
> 

Hi Jakub,

FWIW I don't think this can happen. I didn't mention it because it was 
kind of pointless but I was wrong.

i.e master cannot be NULL here. When cleaning up the link at 
hsr_dellink() we always call timer_delete_sync() for all the background 
jobs before removing the master.

But given this AI report it seems it is just better to remove the master 
check that this patch is introducing to avoid AI reports all over the code.

Thanks,
Fernando.

>> +	}
>> +
>>   	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;
>>