Re: [PATCH net v3] phonet: check register_netdevice_notifier() error in phonet_device_init()

[Andrew Lunn]

On Mon, Jul 20, 2026 at 03:00:31PM +0800, Minhong He wrote:
> phonet_device_init() registers a netdevice notifier before calling
> phonet_netlink_register(), but does not check whether notifier
> registration succeeded. On failure, netlink setup still proceeds and
> init may return success without the notifier in place.
> 
> Also, the existing phonet_netlink_register() failure path called
> phonet_device_exit(), which runs rtnl_unregister_all() even though
> rtnl_register_many() already unwound any partial registration. Calling
> the full exit helper on a partial init is not correct.
> 
> Check each registration error and unwind only the steps that have
> succeeded so far.
> 
> Signed-off-by: Minhong He <heminhong@xxxxxxxxxx>
> ---
> v3:
> - Use goto-based unwind; do not call phonet_device_exit() on
>   phonet_netlink_register() failure (avoids rtnl_unregister_all()
>   after rtnl_register_many() already unwound).
> - Drop Fixes tag (theoretical init failure path; not suitable for
>   stable autosel).
> v2: https://lore.kernel.org/netdev/20260716101504.158387-1-heminhong@xxxxxxxxxx/
> - On notifier registration failure, unwind only proc/pernet.
> v1: https://lore.kernel.org/netdev/20260713075212.431455-1-heminhong@xxxxxxxxxx/
> 
>  net/phonet/pn_dev.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/net/phonet/pn_dev.c b/net/phonet/pn_dev.c
> index ad44831d6745..f41322a12fb7 100644
> --- a/net/phonet/pn_dev.c
> +++ b/net/phonet/pn_dev.c
> @@ -350,16 +350,30 @@ static struct pernet_operations phonet_net_ops = {
>  /* Initialize Phonet devices list */
>  int __init phonet_device_init(void)
>  {
> -	int err = register_pernet_subsys(&phonet_net_ops);
> +	int err;
> +
> +	err = register_pernet_subsys(&phonet_net_ops);
>  	if (err)
>  		return err;
>  
>  	proc_create_net("pnresource", 0, init_net.proc_net, &pn_res_seq_ops,
>  			sizeof(struct seq_net_private));
> -	register_netdevice_notifier(&phonet_device_notifier);
> +
> +	err = register_netdevice_notifier(&phonet_device_notifier);
> +	if (err)
> +		goto err_pernet;
> +
>  	err = phonet_netlink_register();
>  	if (err)
> -		phonet_device_exit();
> +		goto err_notifier;
> +
> +	return 0;
> +
> +err_notifier:
> +	unregister_netdevice_notifier(&phonet_device_notifier);
> +err_pernet:
> +	unregister_pernet_subsys(&phonet_net_ops);
> +	remove_proc_entry("pnresource", init_net.proc_net);

It is good practice to undo in the opposite order to which it was
done. Sometimes there are dependencies, and things will break if you
tear them down in the wrong order.

It is also unusual to see two undo steps without a goto label between
them. Can proc_create_net() fail? Should the return value be tested
and cleanup done if it fails?

    Andrew

---
pw-bot: cr