Re: [PATCH net] batman-adv: check register_netdevice_notifier() and rtnl_link_register() errors

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 2795 bytes --]

On Tuesday, 28 July 2026 05:10:50 CEST Minhong He wrote:
> batadv_init() ignores errors from register_netdevice_notifier() and
> rtnl_link_register(), so the module can load without those registrations
> in place.
> 
> Check both return values and unwind prior initialization in reverse order
> of acquisition on failure.
> 
> Signed-off-by: Minhong He <heminhong@kylinos.cn>
> ---
>  net/batman-adv/main.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

This is not for the net tree/repo. So please don't mark it as such. You must 
target the batadv tree/repo.

> 
> diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
> index 73becb054948..e6bcaf2d14ed 100644
> --- a/net/batman-adv/main.c
> +++ b/net/batman-adv/main.c
> @@ -108,8 +108,14 @@ static int __init batadv_init(void)
>  	if (ret < 0)
>  		goto err_init_wifi;
>  
> -	register_netdevice_notifier(&batadv_hard_if_notifier);
> -	rtnl_link_register(&batadv_link_ops);
> +	ret = register_netdevice_notifier(&batadv_hard_if_notifier);
> +	if (ret < 0)
> +		goto err_reg_notifier;
> +
> +	ret = rtnl_link_register(&batadv_link_ops);
> +	if (ret < 0)
> +		goto err_rtnl_link;
> +
>  	batadv_netlink_register();


You missed the error handling for:

* batadv_netlink_register()
  - doesn't require a cleanup call now but it would be batadv_netlink_unregister()
  - the error must be returned from this function
* batadv_iv_init()
  - no special cleanup call
  - but if you want to add one, please add a deinit wrapper around
    batadv_recv_handler_unregister(BATADV_IV_OGM);
    in bat_iv_ogm.c
* batadv_v_init()
  - no special cleanup call
  - but if you want to add one, please add a deinit wrapper around
    batadv_recv_handler_unregister(BATADV_OGM2);
    batadv_recv_handler_unregister(BATADV_ELP);
    in bat_v.c and make sure you have a dummy variant in bat_v.h

And please adjust the subject to something which doesn't list all the new 
things you check for errors.

>  
>  	pr_info("B.A.T.M.A.N. advanced %s (compatibility version %i) loaded\n",
> @@ -117,6 +123,10 @@ static int __init batadv_init(void)
>  
>  	return 0;
>  
> +err_rtnl_link:
> +	unregister_netdevice_notifier(&batadv_hard_if_notifier);
> +err_reg_notifier:
> +	batadv_wifi_net_devices_deinit();

There must be an rcu_barrier() before batadv_wifi_net_devices_deinit().

>  err_init_wifi:
>  	destroy_workqueue(batadv_event_workqueue);
>  	batadv_event_workqueue = NULL;
> 

See:

* https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/1419594103-10928-6-git-send-email-mpa@pengutronix.de/
* https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20181224174926.20321-1-pakki001@umn.edu/
* https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20221224233311.48678-1-artem.chernyshev@red-soft.ru/

Regards,
	Sven


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]