mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] batman-adv: check register_netdevice_notifier() and rtnl_link_register() errors
@ 2026-07-28  3:10 Minhong He
  2026-07-28  7:04 ` Sven Eckelmann
  2026-07-29  8:44 ` [PATCH v2] batman-adv: handle errors in batadv_init() Minhong He
  0 siblings, 2 replies; 4+ messages in thread
From: Minhong He @ 2026-07-28  3:10 UTC (permalink / raw)
  To: Marek Lindner, Simon Wunderlich, Antonio Quartulli,
	Sven Eckelmann, b.a.t.m.a.n
  Cc: linux-kernel

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

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();
 
 	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();
 err_init_wifi:
 	destroy_workqueue(batadv_event_workqueue);
 	batadv_event_workqueue = NULL;
-- 
2.25.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] batman-adv: check register_netdevice_notifier() and rtnl_link_register() errors
  2026-07-28  3:10 [PATCH net] batman-adv: check register_netdevice_notifier() and rtnl_link_register() errors Minhong He
@ 2026-07-28  7:04 ` Sven Eckelmann
  2026-07-29  8:44 ` [PATCH v2] batman-adv: handle errors in batadv_init() Minhong He
  1 sibling, 0 replies; 4+ messages in thread
From: Sven Eckelmann @ 2026-07-28  7:04 UTC (permalink / raw)
  To: Marek Lindner, Simon Wunderlich, Antonio Quartulli, b.a.t.m.a.n,
	Minhong He
  Cc: linux-kernel, Markus Pargmann, Aditya Pakki, Artem Chernyshev

[-- 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 --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] batman-adv: handle errors in batadv_init()
  2026-07-28  3:10 [PATCH net] batman-adv: check register_netdevice_notifier() and rtnl_link_register() errors Minhong He
  2026-07-28  7:04 ` Sven Eckelmann
@ 2026-07-29  8:44 ` Minhong He
  2026-07-29 15:37   ` Sven Eckelmann
  1 sibling, 1 reply; 4+ messages in thread
From: Minhong He @ 2026-07-29  8:44 UTC (permalink / raw)
  To: Marek Lindner, Simon Wunderlich, Antonio Quartulli,
	Sven Eckelmann, b.a.t.m.a.n
  Cc: linux-kernel

batadv_init() ignores errors from several initialization helpers, so the
module can load without those registrations in place.

Check the fallible init steps and unwind prior initialization in reverse
order of acquisition on failure.

Signed-off-by: Minhong He <heminhong@kylinos.cn>
---
v2:
- Drop the "net" subject prefix; this targets the batman-adv tree
- Also check batadv_v_init()/batadv_iv_init()/batadv_netlink_register()
- Add batadv_v_deinit()/batadv_iv_deinit() helpers for unwind
- Call rcu_barrier() before batadv_wifi_net_devices_deinit()
- Shorten the subject
v1: https://lore.kernel.org/all/20260728031050.76643-1-heminhong@kylinos.cn/

 net/batman-adv/bat_iv_ogm.c |  8 +++++++
 net/batman-adv/bat_iv_ogm.h |  1 +
 net/batman-adv/bat_v.c      |  9 ++++++++
 net/batman-adv/bat_v.h      |  5 ++++
 net/batman-adv/main.c       | 46 +++++++++++++++++++++++++++++--------
 net/batman-adv/netlink.c    |  6 ++++-
 net/batman-adv/netlink.h    |  2 +-
 7 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 22622283f59b..625d4d2ac262 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -2633,3 +2633,11 @@ int __init batadv_iv_init(void)
 out:
 	return ret;
 }
+
+/**
+ * batadv_iv_deinit() - B.A.T.M.A.N. IV deinitialization function
+ */
+void batadv_iv_deinit(void)
+{
+	batadv_recv_handler_unregister(BATADV_IV_OGM);
+}
diff --git a/net/batman-adv/bat_iv_ogm.h b/net/batman-adv/bat_iv_ogm.h
index 04b01bd684e8..90318801aeaf 100644
--- a/net/batman-adv/bat_iv_ogm.h
+++ b/net/batman-adv/bat_iv_ogm.h
@@ -10,5 +10,6 @@
 #include "main.h"
 
 int batadv_iv_init(void);
+void batadv_iv_deinit(void);
 
 #endif /* _NET_BATMAN_ADV_BAT_IV_OGM_H_ */
diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
index db6f5bdcaa98..c1f2d37aa724 100644
--- a/net/batman-adv/bat_v.c
+++ b/net/batman-adv/bat_v.c
@@ -888,3 +888,12 @@ int __init batadv_v_init(void)
 
 	return ret;
 }
+
+/**
+ * batadv_v_deinit() - B.A.T.M.A.N. V deinitialization function
+ */
+void batadv_v_deinit(void)
+{
+	batadv_recv_handler_unregister(BATADV_OGM2);
+	batadv_recv_handler_unregister(BATADV_ELP);
+}
diff --git a/net/batman-adv/bat_v.h b/net/batman-adv/bat_v.h
index 964431f4dc8d..76bd969e79a2 100644
--- a/net/batman-adv/bat_v.h
+++ b/net/batman-adv/bat_v.h
@@ -12,6 +12,7 @@
 #ifdef CONFIG_BATMAN_ADV_BATMAN_V
 
 int batadv_v_init(void);
+void batadv_v_deinit(void);
 void batadv_v_hardif_init(struct batadv_hard_iface *hardif);
 int batadv_v_mesh_init(struct batadv_priv *bat_priv);
 void batadv_v_mesh_free(struct batadv_priv *bat_priv);
@@ -23,6 +24,10 @@ static inline int batadv_v_init(void)
 	return 0;
 }
 
+static inline void batadv_v_deinit(void)
+{
+}
+
 static inline void batadv_v_hardif_init(struct batadv_hard_iface *hardif)
 {
 }
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 73becb054948..8fe8a2b27a74 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -94,35 +94,63 @@ static int __init batadv_init(void)
 
 	batadv_recv_handler_init();
 
-	batadv_v_init();
-	batadv_iv_init();
+	ret = batadv_v_init();
+	if (ret < 0)
+		goto err_tt;
+
+	ret = batadv_iv_init();
+	if (ret < 0)
+		goto err_v;
+
 	batadv_tp_meter_init();
 
 	batadv_event_workqueue = create_singlethread_workqueue("bat_events");
 	if (!batadv_event_workqueue) {
 		ret = -ENOMEM;
-		goto err_create_wq;
+		goto err_iv;
 	}
 
 	ret = batadv_wifi_net_devices_init();
 	if (ret < 0)
-		goto err_init_wifi;
+		goto err_wq;
 
-	register_netdevice_notifier(&batadv_hard_if_notifier);
-	rtnl_link_register(&batadv_link_ops);
-	batadv_netlink_register();
+	ret = register_netdevice_notifier(&batadv_hard_if_notifier);
+	if (ret < 0)
+		goto err_wifi;
+
+	ret = rtnl_link_register(&batadv_link_ops);
+	if (ret < 0)
+		goto err_notifier;
+
+	ret = batadv_netlink_register();
+	if (ret < 0)
+		goto err_rtnl;
 
 	pr_info("B.A.T.M.A.N. advanced %s (compatibility version %i) loaded\n",
 		init_utsname()->release, BATADV_COMPAT_VERSION);
 
 	return 0;
 
-err_init_wifi:
+err_rtnl:
+	rtnl_link_unregister(&batadv_link_ops);
+err_notifier:
+	unregister_netdevice_notifier(&batadv_hard_if_notifier);
+err_wifi:
 	destroy_workqueue(batadv_event_workqueue);
 	batadv_event_workqueue = NULL;
 	rcu_barrier();
+	batadv_wifi_net_devices_deinit();
+	goto err_iv;
 
-err_create_wq:
+err_wq:
+	destroy_workqueue(batadv_event_workqueue);
+	batadv_event_workqueue = NULL;
+	rcu_barrier();
+err_iv:
+	batadv_iv_deinit();
+err_v:
+	batadv_v_deinit();
+err_tt:
 	batadv_tt_cache_destroy();
 
 	return ret;
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index d2bc48c70714..99341f39df2a 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -1550,14 +1550,18 @@ struct genl_family batadv_netlink_family __ro_after_init = {
 
 /**
  * batadv_netlink_register() - register batadv genl netlink family
+ *
+ * Return: 0 on success or negative error number in case of failure
  */
-void __init batadv_netlink_register(void)
+int __init batadv_netlink_register(void)
 {
 	int ret;
 
 	ret = genl_register_family(&batadv_netlink_family);
 	if (ret)
 		pr_warn("unable to register netlink family\n");
+
+	return ret;
 }
 
 /**
diff --git a/net/batman-adv/netlink.h b/net/batman-adv/netlink.h
index 4eae9e5ff135..f92d3ea7d06c 100644
--- a/net/batman-adv/netlink.h
+++ b/net/batman-adv/netlink.h
@@ -12,7 +12,7 @@
 #include <linux/netlink.h>
 #include <linux/types.h>
 
-void batadv_netlink_register(void);
+int batadv_netlink_register(void);
 void batadv_netlink_unregister(void);
 struct net_device *batadv_netlink_get_meshif(struct netlink_callback *cb);
 struct batadv_hard_iface *
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] batman-adv: handle errors in batadv_init()
  2026-07-29  8:44 ` [PATCH v2] batman-adv: handle errors in batadv_init() Minhong He
@ 2026-07-29 15:37   ` Sven Eckelmann
  0 siblings, 0 replies; 4+ messages in thread
From: Sven Eckelmann @ 2026-07-29 15:37 UTC (permalink / raw)
  To: Minhong He
  Cc: Marek Lindner, Simon Wunderlich, Antonio Quartulli,
	Sven Eckelmann, b.a.t.m.a.n, linux-kernel

On Wed, 29 Jul 2026 16:44:35 +0800, Minhong He <heminhong@kylinos.cn> wrote:
> batadv_init() ignores errors from several initialization helpers, so the
> module can load without those registrations in place.
> 
> Check the fallible init steps and unwind prior initialization in reverse
> order of acquisition on failure.
> 
> Signed-off-by: Minhong He <heminhong@kylinos.cn>

Please don't send new versions as reply to the old version.

"batadv" would be correct prefix for the batadv.git repo at
https://git.open-mesh.org/batadv.git. So something like "[PATCH batadv v2]"

>
>
> diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
> index 73becb0549488..8fe8a2b27a744 100644
> --- a/net/batman-adv/main.c
> +++ b/net/batman-adv/main.c
> @@ -94,35 +94,63 @@ static int __init batadv_init(void)
> [ ... skip 52 lines ... ]
> +err_wifi:
>  	destroy_workqueue(batadv_event_workqueue);
>  	batadv_event_workqueue = NULL;
>  	rcu_barrier();
> +	batadv_wifi_net_devices_deinit();
> +	goto err_iv;

No, this is hard to see an will attract the goto raptor.

Feel free to move the "ret = batadv_wifi_net_devices_init();" before the
"batadv_event_workqueue = create_singlethread_workqueue("bat_events");" to keep
the reverse deinit order and only have a single rcu_barrier().

-- 
Sven Eckelmann <sven@narfation.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-29 15:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-28  3:10 [PATCH net] batman-adv: check register_netdevice_notifier() and rtnl_link_register() errors Minhong He
2026-07-28  7:04 ` Sven Eckelmann
2026-07-29  8:44 ` [PATCH v2] batman-adv: handle errors in batadv_init() Minhong He
2026-07-29 15:37   ` Sven Eckelmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®