[PATCH v1 net 0/6] rtnetlink: Handle error of rtnl_register_module().

[PATCH v1 net 0/6] rtnetlink: Handle error of rtnl_register_module().

[Kuniyuki Iwashima]

While converting phonet to per-netns RTNL, I found a weird comment

  /* Further rtnl_register_module() cannot fail */

that was true but no longer true after commit addf9b90de22 ("net:
rtnetlink: use rcu to free rtnl message handlers").

Many callers of rtnl_register_module() just ignore the returned
value but should handle them properly.

This series introduces two helpers, rtnl_register_module_many() and
rtnl_unregister_module_many(), to do that easily and fix such callers.

All rtnl_register() and rtnl_register_module() will be converted
to _many() variant and some rtnl_lock() will be saved in _many()
later in net-next.


Kuniyuki Iwashima (6):
  rtnetlink: Add bulk registration helpers for rtnetlink message
    handlers.
  vxlan: Handle error of rtnl_register_module().
  bridge: Handle error of rtnl_register_module().
  mctp: Handle error of rtnl_register_module().
  mpls: Handle error of rtnl_register_module().
  phonet: Handle error of rtnl_register_module().

 drivers/net/vxlan/vxlan_core.c      |  6 ++++-
 drivers/net/vxlan/vxlan_private.h   |  2 +-
 drivers/net/vxlan/vxlan_vnifilter.c | 19 ++++++++--------
 include/net/mctp.h                  |  2 +-
 include/net/rtnetlink.h             | 19 ++++++++++++++++
 net/bridge/br_netlink.c             |  6 ++++-
 net/bridge/br_private.h             |  5 +++--
 net/bridge/br_vlan.c                | 19 ++++++++--------
 net/core/rtnetlink.c                | 30 +++++++++++++++++++++++++
 net/mctp/af_mctp.c                  |  6 ++++-
 net/mctp/device.c                   | 30 +++++++++++++++----------
 net/mctp/neigh.c                    | 29 ++++++++++++++----------
 net/mctp/route.c                    | 34 ++++++++++++++++++++---------
 net/mpls/af_mpls.c                  | 32 +++++++++++++++++----------
 net/phonet/pn_netlink.c             | 27 +++++++++--------------
 15 files changed, 178 insertions(+), 88 deletions(-)

-- 
2.30.2

[PATCH v1 net 1/6] rtnetlink: Add bulk registration helpers for rtnetlink message handlers.

[Kuniyuki Iwashima]

Before commit addf9b90de22 ("net: rtnetlink: use rcu to free rtnl message
handlers"), once rtnl_msg_handlers[protocol] was allocated, the following
rtnl_register_module() for the same protocol never failed.

However, after the commit, rtnl_msg_handler[protocol][msgtype] needs to
be allocated in each rtnl_register_module(), so each call could fail.

Many callers of rtnl_register_module() do not handle the returned error,
and we need to add many error handlings.

To handle that easily, let's add wrapper functions for bulk registration
of rtnetlink message handlers.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/rtnetlink.h | 19 +++++++++++++++++++
 net/core/rtnetlink.c    | 30 ++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index b45d57b5968a..b6b91898dc13 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -29,6 +29,14 @@ static inline enum rtnl_kinds rtnl_msgtype_kind(int msgtype)
 	return msgtype & RTNL_KIND_MASK;
 }
 
+struct rtnl_msg_handler {
+	int protocol;
+	int msgtype;
+	rtnl_doit_func doit;
+	rtnl_dumpit_func dumpit;
+	int flags;
+};
+
 void rtnl_register(int protocol, int msgtype,
 		   rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
 int rtnl_register_module(struct module *owner, int protocol, int msgtype,
@@ -36,6 +44,17 @@ int rtnl_register_module(struct module *owner, int protocol, int msgtype,
 int rtnl_unregister(int protocol, int msgtype);
 void rtnl_unregister_all(int protocol);
 
+int __rtnl_register_many(struct module *owner,
+			 struct rtnl_msg_handler *handlers, int n);
+void __rtnl_unregister_many(struct rtnl_msg_handler *handlers, int n);
+
+#define rtnl_register_many(handlers)						\
+	__rtnl_register_many(NULL, handlers, ARRAY_SIZE(handlers))
+#define rtnl_register_module_many(handlers)					\
+	__rtnl_register_many(THIS_MODULE, handlers, ARRAY_SIZE(handlers))
+#define rtnl_unregister_many(handlers)						\
+	__rtnl_unregister_many(handlers, ARRAY_SIZE(handlers))
+
 static inline int rtnl_msg_family(const struct nlmsghdr *nlh)
 {
 	if (nlmsg_len(nlh) >= sizeof(struct rtgenmsg))
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index f0a520987085..1fac9e786315 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -384,6 +384,36 @@ void rtnl_unregister_all(int protocol)
 }
 EXPORT_SYMBOL_GPL(rtnl_unregister_all);
 
+int __rtnl_register_many(struct module *owner,
+			 struct rtnl_msg_handler *handlers, int n)
+{
+	struct rtnl_msg_handler *handler;
+	int i, err;
+
+	for (i = 0, handler = handlers; i < n; i++, handler++) {
+		err = rtnl_register_internal(owner, handler->protocol,
+					     handler->msgtype, handler->doit,
+					     handler->dumpit, handler->flags);
+		if (err) {
+			__rtnl_unregister_many(handlers, i);
+			break;
+		}
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(__rtnl_register_many);
+
+void __rtnl_unregister_many(struct rtnl_msg_handler *handlers, int n)
+{
+	struct rtnl_msg_handler *handler;
+	int i;
+
+	for (i = n - 1, handler = handlers + n - 1; i >= 0; i--, handler--)
+		rtnl_unregister(handler->protocol, handler->msgtype);
+}
+EXPORT_SYMBOL_GPL(__rtnl_unregister_many);
+
 static LIST_HEAD(link_ops);
 
 static const struct rtnl_link_ops *rtnl_link_ops_get(const char *kind)
-- 
2.30.2

[PATCH v1 net 2/6] vxlan: Handle error of rtnl_register_module().

[Kuniyuki Iwashima]

Since introduced, vxlan_vnifilter_init() has been ignoring the
returned value of rtnl_register_module(), which could fail.

Let's handle the errors by rtnl_register_module_many().

Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Roopa Prabhu <roopa@nvidia.com>
---
 drivers/net/vxlan/vxlan_core.c      |  6 +++++-
 drivers/net/vxlan/vxlan_private.h   |  2 +-
 drivers/net/vxlan/vxlan_vnifilter.c | 19 +++++++++----------
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 53dcb9fffc04..6e9a3795846a 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -4913,9 +4913,13 @@ static int __init vxlan_init_module(void)
 	if (rc)
 		goto out4;
 
-	vxlan_vnifilter_init();
+	rc = vxlan_vnifilter_init();
+	if (rc)
+		goto out5;
 
 	return 0;
+out5:
+	rtnl_link_unregister(&vxlan_link_ops);
 out4:
 	unregister_switchdev_notifier(&vxlan_switchdev_notifier_block);
 out3:
diff --git a/drivers/net/vxlan/vxlan_private.h b/drivers/net/vxlan/vxlan_private.h
index b35d96b78843..76a351a997d5 100644
--- a/drivers/net/vxlan/vxlan_private.h
+++ b/drivers/net/vxlan/vxlan_private.h
@@ -202,7 +202,7 @@ int vxlan_vni_in_use(struct net *src_net, struct vxlan_dev *vxlan,
 int vxlan_vnigroup_init(struct vxlan_dev *vxlan);
 void vxlan_vnigroup_uninit(struct vxlan_dev *vxlan);
 
-void vxlan_vnifilter_init(void);
+int vxlan_vnifilter_init(void);
 void vxlan_vnifilter_uninit(void);
 void vxlan_vnifilter_count(struct vxlan_dev *vxlan, __be32 vni,
 			   struct vxlan_vni_node *vninode,
diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index 9c59d0bf8c3d..4be5d3060039 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -992,19 +992,18 @@ static int vxlan_vnifilter_process(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return err;
 }
 
-void vxlan_vnifilter_init(void)
+static struct rtnl_msg_handler vxlan_rtnl_msg_handlers[] = {
+	{PF_BRIDGE, RTM_GETTUNNEL, NULL, vxlan_vnifilter_dump, 0},
+	{PF_BRIDGE, RTM_NEWTUNNEL, vxlan_vnifilter_process, NULL, 0},
+	{PF_BRIDGE, RTM_DELTUNNEL, vxlan_vnifilter_process, NULL, 0},
+};
+
+int vxlan_vnifilter_init(void)
 {
-	rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_GETTUNNEL, NULL,
-			     vxlan_vnifilter_dump, 0);
-	rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_NEWTUNNEL,
-			     vxlan_vnifilter_process, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_DELTUNNEL,
-			     vxlan_vnifilter_process, NULL, 0);
+	return rtnl_register_module_many(vxlan_rtnl_msg_handlers);
 }
 
 void vxlan_vnifilter_uninit(void)
 {
-	rtnl_unregister(PF_BRIDGE, RTM_GETTUNNEL);
-	rtnl_unregister(PF_BRIDGE, RTM_NEWTUNNEL);
-	rtnl_unregister(PF_BRIDGE, RTM_DELTUNNEL);
+	rtnl_unregister_many(vxlan_rtnl_msg_handlers);
 }
-- 
2.30.2

[PATCH v1 net 3/6] bridge: Handle error of rtnl_register_module().

[Kuniyuki Iwashima]

Since introduced, br_vlan_rtnl_init() has been ignoring the returned
value of rtnl_register_module(), which could fail.

Let's handle the errors by rtnl_register_module_many().

Fixes: 8dcea187088b ("net: bridge: vlan: add rtm definitions and dump support")
Fixes: f26b296585dc ("net: bridge: vlan: add new rtm message support")
Fixes: adb3ce9bcb0f ("net: bridge: vlan: add del rtm message support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Roopa Prabhu <roopa@nvidia.com>
Cc: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_netlink.c |  6 +++++-
 net/bridge/br_private.h |  5 +++--
 net/bridge/br_vlan.c    | 19 +++++++++----------
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index f17dbac7d828..6b97ae47f855 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1920,7 +1920,10 @@ int __init br_netlink_init(void)
 {
 	int err;
 
-	br_vlan_rtnl_init();
+	err = br_vlan_rtnl_init();
+	if (err)
+		goto out;
+
 	rtnl_af_register(&br_af_ops);
 
 	err = rtnl_link_register(&br_link_ops);
@@ -1931,6 +1934,7 @@ int __init br_netlink_init(void)
 
 out_af:
 	rtnl_af_unregister(&br_af_ops);
+out:
 	return err;
 }
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index d4bedc87b1d8..041f6e571a20 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1571,7 +1571,7 @@ void br_vlan_get_stats(const struct net_bridge_vlan *v,
 void br_vlan_port_event(struct net_bridge_port *p, unsigned long event);
 int br_vlan_bridge_event(struct net_device *dev, unsigned long event,
 			 void *ptr);
-void br_vlan_rtnl_init(void);
+int br_vlan_rtnl_init(void);
 void br_vlan_rtnl_uninit(void);
 void br_vlan_notify(const struct net_bridge *br,
 		    const struct net_bridge_port *p,
@@ -1802,8 +1802,9 @@ static inline int br_vlan_bridge_event(struct net_device *dev,
 	return 0;
 }
 
-static inline void br_vlan_rtnl_init(void)
+static inline int br_vlan_rtnl_init(void)
 {
+	return 0;
 }
 
 static inline void br_vlan_rtnl_uninit(void)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 9c2fffb827ab..a9f00d72fe18 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -2296,19 +2296,18 @@ static int br_vlan_rtm_process(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return err;
 }
 
-void br_vlan_rtnl_init(void)
+static struct rtnl_msg_handler br_vlan_rtnl_msg_handlers[] = {
+	{PF_BRIDGE, RTM_NEWVLAN, br_vlan_rtm_process, NULL, 0},
+	{PF_BRIDGE, RTM_DELVLAN, br_vlan_rtm_process, NULL, 0},
+	{PF_BRIDGE, RTM_GETVLAN, NULL, br_vlan_rtm_dump, 0},
+};
+
+int br_vlan_rtnl_init(void)
 {
-	rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_GETVLAN, NULL,
-			     br_vlan_rtm_dump, 0);
-	rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_NEWVLAN,
-			     br_vlan_rtm_process, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_DELVLAN,
-			     br_vlan_rtm_process, NULL, 0);
+	return rtnl_register_module_many(br_vlan_rtnl_msg_handlers);
 }
 
 void br_vlan_rtnl_uninit(void)
 {
-	rtnl_unregister(PF_BRIDGE, RTM_GETVLAN);
-	rtnl_unregister(PF_BRIDGE, RTM_NEWVLAN);
-	rtnl_unregister(PF_BRIDGE, RTM_DELVLAN);
+	rtnl_unregister_many(br_vlan_rtnl_msg_handlers);
 }
-- 
2.30.2

[PATCH v1 net 4/6] mctp: Handle error of rtnl_register_module().

[Kuniyuki Iwashima]

Since introduced, mctp has been ignoring the returned value
of rtnl_register_module(), which could fail.

Let's handle the errors by rtnl_register_module_many().

Fixes: 583be982d934 ("mctp: Add device handling and netlink interface")
Fixes: 831119f88781 ("mctp: Add neighbour netlink interface")
Fixes: 06d2f4c583a7 ("mctp: Add netlink route management")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Jeremy Kerr <jk@codeconstruct.com.au>
Cc: Matt Johnston <matt@codeconstruct.com.au>
---
 include/net/mctp.h |  2 +-
 net/mctp/af_mctp.c |  6 +++++-
 net/mctp/device.c  | 30 ++++++++++++++++++------------
 net/mctp/neigh.c   | 29 ++++++++++++++++++-----------
 net/mctp/route.c   | 34 ++++++++++++++++++++++++----------
 5 files changed, 66 insertions(+), 35 deletions(-)

diff --git a/include/net/mctp.h b/include/net/mctp.h
index 7b17c52e8ce2..28d59ae94ca3 100644
--- a/include/net/mctp.h
+++ b/include/net/mctp.h
@@ -295,7 +295,7 @@ void mctp_neigh_remove_dev(struct mctp_dev *mdev);
 int mctp_routes_init(void);
 void mctp_routes_exit(void);
 
-void mctp_device_init(void);
+int mctp_device_init(void);
 void mctp_device_exit(void);
 
 #endif /* __NET_MCTP_H */
diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c
index 43288b408fde..f6de136008f6 100644
--- a/net/mctp/af_mctp.c
+++ b/net/mctp/af_mctp.c
@@ -756,10 +756,14 @@ static __init int mctp_init(void)
 	if (rc)
 		goto err_unreg_routes;
 
-	mctp_device_init();
+	rc = mctp_device_init();
+	if (rc)
+		goto err_unreg_neigh;
 
 	return 0;
 
+err_unreg_neigh:
+	mctp_neigh_exit();
 err_unreg_routes:
 	mctp_routes_exit();
 err_unreg_proto:
diff --git a/net/mctp/device.c b/net/mctp/device.c
index acb97b257428..3e5cfe119519 100644
--- a/net/mctp/device.c
+++ b/net/mctp/device.c
@@ -524,25 +524,31 @@ static struct notifier_block mctp_dev_nb = {
 	.priority = ADDRCONF_NOTIFY_PRIORITY,
 };
 
-void __init mctp_device_init(void)
+static struct rtnl_msg_handler mctp_rtnl_addr_msg_handlers[] = {
+	{PF_MCTP, RTM_NEWADDR, mctp_rtm_newaddr, NULL, 0},
+	{PF_MCTP, RTM_DELADDR, mctp_rtm_deladdr, NULL, 0},
+	{PF_MCTP, RTM_GETADDR, NULL, mctp_dump_addrinfo, 0},
+};
+
+int __init mctp_device_init(void)
 {
-	register_netdevice_notifier(&mctp_dev_nb);
+	int err;
 
-	rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_GETADDR,
-			     NULL, mctp_dump_addrinfo, 0);
-	rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_NEWADDR,
-			     mctp_rtm_newaddr, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_DELADDR,
-			     mctp_rtm_deladdr, NULL, 0);
+	register_netdevice_notifier(&mctp_dev_nb);
 	rtnl_af_register(&mctp_af_ops);
+
+	err = rtnl_register_module_many(mctp_rtnl_addr_msg_handlers);
+	if (err) {
+		rtnl_af_unregister(&mctp_af_ops);
+		unregister_netdevice_notifier(&mctp_dev_nb);
+	}
+
+	return err;
 }
 
 void __exit mctp_device_exit(void)
 {
+	rtnl_unregister_many(mctp_rtnl_addr_msg_handlers);
 	rtnl_af_unregister(&mctp_af_ops);
-	rtnl_unregister(PF_MCTP, RTM_DELADDR);
-	rtnl_unregister(PF_MCTP, RTM_NEWADDR);
-	rtnl_unregister(PF_MCTP, RTM_GETADDR);
-
 	unregister_netdevice_notifier(&mctp_dev_nb);
 }
diff --git a/net/mctp/neigh.c b/net/mctp/neigh.c
index ffa0f9e0983f..6bcdae3c53b2 100644
--- a/net/mctp/neigh.c
+++ b/net/mctp/neigh.c
@@ -322,22 +322,29 @@ static struct pernet_operations mctp_net_ops = {
 	.exit = mctp_neigh_net_exit,
 };
 
+static struct rtnl_msg_handler mctp_rtnl_neigh_msg_handlers[] = {
+	{PF_MCTP, RTM_NEWNEIGH, mctp_rtm_newneigh, NULL, 0},
+	{PF_MCTP, RTM_DELNEIGH, mctp_rtm_delneigh, NULL, 0},
+	{PF_MCTP, RTM_GETNEIGH, NULL, mctp_rtm_getneigh, 0},
+};
+
 int __init mctp_neigh_init(void)
 {
-	rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_NEWNEIGH,
-			     mctp_rtm_newneigh, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_DELNEIGH,
-			     mctp_rtm_delneigh, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_GETNEIGH,
-			     NULL, mctp_rtm_getneigh, 0);
-
-	return register_pernet_subsys(&mctp_net_ops);
+	int err;
+
+	err = rtnl_register_module_many(mctp_rtnl_neigh_msg_handlers);
+	if (err)
+		return err;
+
+	err = register_pernet_subsys(&mctp_net_ops);
+	if (err)
+		rtnl_unregister_many(mctp_rtnl_neigh_msg_handlers);
+
+	return err;
 }
 
 void __exit mctp_neigh_exit(void)
 {
 	unregister_pernet_subsys(&mctp_net_ops);
-	rtnl_unregister(PF_MCTP, RTM_GETNEIGH);
-	rtnl_unregister(PF_MCTP, RTM_DELNEIGH);
-	rtnl_unregister(PF_MCTP, RTM_NEWNEIGH);
+	rtnl_unregister_many(mctp_rtnl_neigh_msg_handlers);
 }
diff --git a/net/mctp/route.c b/net/mctp/route.c
index eefd7834d9a0..88ac6feb4e22 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -1474,26 +1474,40 @@ static struct pernet_operations mctp_net_ops = {
 	.exit = mctp_routes_net_exit,
 };
 
+static struct rtnl_msg_handler mctp_rtnl_route_msg_handlers[] = {
+	{PF_MCTP, RTM_NEWROUTE, mctp_newroute, NULL, 0},
+	{PF_MCTP, RTM_DELROUTE, mctp_delroute, NULL, 0},
+	{PF_MCTP, RTM_GETROUTE, NULL, mctp_dump_rtinfo, 0},
+};
+
 int __init mctp_routes_init(void)
 {
+	int err;
+
 	dev_add_pack(&mctp_packet_type);
 
-	rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_GETROUTE,
-			     NULL, mctp_dump_rtinfo, 0);
-	rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_NEWROUTE,
-			     mctp_newroute, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_DELROUTE,
-			     mctp_delroute, NULL, 0);
+	err = rtnl_register_module_many(mctp_rtnl_route_msg_handlers);
+	if (err)
+		goto fail_rtnl;
+
+	err = register_pernet_subsys(&mctp_net_ops);
+	if (err)
+		goto fail_pernet;
 
-	return register_pernet_subsys(&mctp_net_ops);
+out:
+	return err;
+
+fail_pernet:
+	rtnl_unregister_many(mctp_rtnl_route_msg_handlers);
+fail_rtnl:
+	dev_remove_pack(&mctp_packet_type);
+	goto out;
 }
 
 void mctp_routes_exit(void)
 {
 	unregister_pernet_subsys(&mctp_net_ops);
-	rtnl_unregister(PF_MCTP, RTM_DELROUTE);
-	rtnl_unregister(PF_MCTP, RTM_NEWROUTE);
-	rtnl_unregister(PF_MCTP, RTM_GETROUTE);
+	rtnl_unregister_many(mctp_rtnl_route_msg_handlers);
 	dev_remove_pack(&mctp_packet_type);
 }
 
-- 
2.30.2

Re: [PATCH v1 net 4/6] mctp: Handle error of rtnl_register_module().

[Jakub Kicinski]

On Thu, 3 Oct 2024 13:57:23 -0700 Kuniyuki Iwashima wrote:
> Since introduced, mctp has been ignoring the returned value
> of rtnl_register_module(), which could fail.
> 
> Let's handle the errors by rtnl_register_module_many().
> 
> Fixes: 583be982d934 ("mctp: Add device handling and netlink interface")
> Fixes: 831119f88781 ("mctp: Add neighbour netlink interface")
> Fixes: 06d2f4c583a7 ("mctp: Add netlink route management")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Build's unhappy about the section markings:

WARNING: modpost: vmlinux: section mismatch in reference: mctp_init+0xb7 (section: .init.text) -> mctp_neigh_exit (section: .exit.text)
-- 
pw-bot: cr

Re: [PATCH v1 net 4/6] mctp: Handle error of rtnl_register_module().

[Kuniyuki Iwashima]

From: Jakub Kicinski <kuba@kernel.org>
Date: Thu, 3 Oct 2024 17:39:28 -0700
> On Thu, 3 Oct 2024 13:57:23 -0700 Kuniyuki Iwashima wrote:
> > Since introduced, mctp has been ignoring the returned value
> > of rtnl_register_module(), which could fail.
> > 
> > Let's handle the errors by rtnl_register_module_many().
> > 
> > Fixes: 583be982d934 ("mctp: Add device handling and netlink interface")
> > Fixes: 831119f88781 ("mctp: Add neighbour netlink interface")
> > Fixes: 06d2f4c583a7 ("mctp: Add netlink route management")
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> 
> Build's unhappy about the section markings:
> 
> WARNING: modpost: vmlinux: section mismatch in reference: mctp_init+0xb7 (section: .init.text) -> mctp_neigh_exit (section: .exit.text)

Thanks for catching, will remove __exit from mctp_neigh_exit().

BTW what option is needed to reproduce it ?
I tried W=1 C=1 but didn't see that warning.

Re: [PATCH v1 net 4/6] mctp: Handle error of rtnl_register_module().

[Jakub Kicinski]

On Thu, 3 Oct 2024 18:00:48 -0700 Kuniyuki Iwashima wrote:
> BTW what option is needed to reproduce it ?
> I tried W=1 C=1 but didn't see that warning.

Are you doing allmodconfig?
I think it may be a Kconfig option. DEBUG_SECTION_MISMATCH ?

Re: [PATCH v1 net 4/6] mctp: Handle error of rtnl_register_module().

[Kuniyuki Iwashima]

From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Oct 2024 09:06:08 -0700
> On Thu, 3 Oct 2024 18:00:48 -0700 Kuniyuki Iwashima wrote:
> > BTW what option is needed to reproduce it ?
> > I tried W=1 C=1 but didn't see that warning.
> 
> Are you doing allmodconfig?
> I think it may be a Kconfig option. DEBUG_SECTION_MISMATCH ?

TIL DEBUG_SECTION_MISMATCH :)

now I see the warning, thanks !

[PATCH v1 net 5/6] mpls: Handle error of rtnl_register_module().

[Kuniyuki Iwashima]

Since introduced, mpls_init() has been ignoring the returned
value of rtnl_register_module(), which could fail.

Let's handle the errors by rtnl_register_module_many().

Fixes: 03c0566542f4 ("mpls: Netlink commands to add, remove, and dump routes")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
---
 net/mpls/af_mpls.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index aba983531ed3..653897327165 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -2728,6 +2728,15 @@ static struct rtnl_af_ops mpls_af_ops __read_mostly = {
 	.get_stats_af_size = mpls_get_stats_af_size,
 };
 
+static struct rtnl_msg_handler mpls_rtnl_msg_handlers[] = {
+	{PF_MPLS, RTM_NEWROUTE, mpls_rtm_newroute, NULL, 0},
+	{PF_MPLS, RTM_DELROUTE, mpls_rtm_delroute, NULL, 0},
+	{PF_MPLS, RTM_GETROUTE, mpls_getroute, mpls_dump_routes, 0},
+	{PF_MPLS, RTM_GETNETCONF,
+	 mpls_netconf_get_devconf, mpls_netconf_dump_devconf,
+	 RTNL_FLAG_DUMP_UNLOCKED},
+};
+
 static int __init mpls_init(void)
 {
 	int err;
@@ -2746,24 +2755,25 @@ static int __init mpls_init(void)
 
 	rtnl_af_register(&mpls_af_ops);
 
-	rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_NEWROUTE,
-			     mpls_rtm_newroute, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_DELROUTE,
-			     mpls_rtm_delroute, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_GETROUTE,
-			     mpls_getroute, mpls_dump_routes, 0);
-	rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_GETNETCONF,
-			     mpls_netconf_get_devconf,
-			     mpls_netconf_dump_devconf,
-			     RTNL_FLAG_DUMP_UNLOCKED);
-	err = ipgre_tunnel_encap_add_mpls_ops();
+	err = rtnl_register_module_many(mpls_rtnl_msg_handlers);
 	if (err)
+		goto out_unregister_rtnl_af;
+
+	err = ipgre_tunnel_encap_add_mpls_ops();
+	if (err) {
 		pr_err("Can't add mpls over gre tunnel ops\n");
+		goto out_unregister_rtnl;
+	}
 
 	err = 0;
 out:
 	return err;
 
+out_unregister_rtnl:
+	rtnl_unregister_many(mpls_rtnl_msg_handlers);
+out_unregister_rtnl_af:
+	rtnl_af_unregister(&mpls_af_ops);
+	dev_remove_pack(&mpls_packet_type);
 out_unregister_pernet:
 	unregister_pernet_subsys(&mpls_net_ops);
 	goto out;
-- 
2.30.2

[PATCH v1 net 6/6] phonet: Handle error of rtnl_register_module().

[Kuniyuki Iwashima]

Before commit addf9b90de22 ("net: rtnetlink: use rcu to free rtnl
message handlers"), once the first rtnl_register_module() allocated
rtnl_msg_handlers[PF_PHONET], the following calls never failed.

However, after the commit, rtnl_register_module() could fail to allocate
rtnl_msg_handlers[PF_PHONET][msgtype] and requires error handling for
each call.

Let's use rtnl_register_module_many() to handle the errors easily.

Fixes: addf9b90de22 ("net: rtnetlink: use rcu to free rtnl message handlers")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Remi Denis-Courmont <courmisch@gmail.com>
Cc: Florian Westphal <fw@strlen.de>
---
 net/phonet/pn_netlink.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/net/phonet/pn_netlink.c b/net/phonet/pn_netlink.c
index 7008d402499d..d39e6983926b 100644
--- a/net/phonet/pn_netlink.c
+++ b/net/phonet/pn_netlink.c
@@ -285,23 +285,16 @@ static int route_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	return err;
 }
 
+static struct rtnl_msg_handler phonet_rtnl_msg_handlers[] __initdata_or_module = {
+	{PF_PHONET, RTM_NEWADDR, addr_doit, NULL, 0},
+	{PF_PHONET, RTM_DELADDR, addr_doit, NULL, 0},
+	{PF_PHONET, RTM_GETADDR, NULL, getaddr_dumpit, 0},
+	{PF_PHONET, RTM_NEWROUTE, route_doit, NULL, 0},
+	{PF_PHONET, RTM_DELROUTE, route_doit, NULL, 0},
+	{PF_PHONET, RTM_GETROUTE, NULL, route_dumpit, RTNL_FLAG_DUMP_UNLOCKED},
+};
+
 int __init phonet_netlink_register(void)
 {
-	int err = rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_NEWADDR,
-				       addr_doit, NULL, 0);
-	if (err)
-		return err;
-
-	/* Further rtnl_register_module() cannot fail */
-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_DELADDR,
-			     addr_doit, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_GETADDR,
-			     NULL, getaddr_dumpit, 0);
-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_NEWROUTE,
-			     route_doit, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_DELROUTE,
-			     route_doit, NULL, 0);
-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_GETROUTE,
-			     NULL, route_dumpit, RTNL_FLAG_DUMP_UNLOCKED);
-	return 0;
+	return rtnl_register_module_many(phonet_rtnl_msg_handlers);
 }
-- 
2.30.2

Re: [PATCH v1 net 6/6] phonet: Handle error of rtnl_register_module().

[Rémi Denis-Courmont]

Le 4 octobre 2024 05:57:25 GMT+09:00, Kuniyuki Iwashima <kuniyu@amazon.com> a écrit :
>Before commit addf9b90de22 ("net: rtnetlink: use rcu to free rtnl
>message handlers"), once the first rtnl_register_module() allocated
>rtnl_msg_handlers[PF_PHONET], the following calls never failed.
>
>However, after the commit, rtnl_register_module() could fail to allocate
>rtnl_msg_handlers[PF_PHONET][msgtype] and requires error handling for
>each call.
>
>Let's use rtnl_register_module_many() to handle the errors easily.
>
>Fixes: addf9b90de22 ("net: rtnetlink: use rcu to free rtnl message handlers")
>Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Acked-by: Rémi Denis-Courmont <courmisch@gmail.com>

>---
>Cc: Remi Denis-Courmont <courmisch@gmail.com>
>Cc: Florian Westphal <fw@strlen.de>
>---
> net/phonet/pn_netlink.c | 27 ++++++++++-----------------
> 1 file changed, 10 insertions(+), 17 deletions(-)
>
>diff --git a/net/phonet/pn_netlink.c b/net/phonet/pn_netlink.c
>index 7008d402499d..d39e6983926b 100644
>--- a/net/phonet/pn_netlink.c
>+++ b/net/phonet/pn_netlink.c
>@@ -285,23 +285,16 @@ static int route_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
> 	return err;
> }
> 
>+static struct rtnl_msg_handler phonet_rtnl_msg_handlers[] __initdata_or_module = {
>+	{PF_PHONET, RTM_NEWADDR, addr_doit, NULL, 0},
>+	{PF_PHONET, RTM_DELADDR, addr_doit, NULL, 0},
>+	{PF_PHONET, RTM_GETADDR, NULL, getaddr_dumpit, 0},
>+	{PF_PHONET, RTM_NEWROUTE, route_doit, NULL, 0},
>+	{PF_PHONET, RTM_DELROUTE, route_doit, NULL, 0},
>+	{PF_PHONET, RTM_GETROUTE, NULL, route_dumpit, RTNL_FLAG_DUMP_UNLOCKED},
>+};
>+
> int __init phonet_netlink_register(void)
> {
>-	int err = rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_NEWADDR,
>-				       addr_doit, NULL, 0);
>-	if (err)
>-		return err;
>-
>-	/* Further rtnl_register_module() cannot fail */
>-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_DELADDR,
>-			     addr_doit, NULL, 0);
>-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_GETADDR,
>-			     NULL, getaddr_dumpit, 0);
>-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_NEWROUTE,
>-			     route_doit, NULL, 0);
>-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_DELROUTE,
>-			     route_doit, NULL, 0);
>-	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_GETROUTE,
>-			     NULL, route_dumpit, RTNL_FLAG_DUMP_UNLOCKED);
>-	return 0;
>+	return rtnl_register_module_many(phonet_rtnl_msg_handlers);
> }