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