| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b82788e2fb368b31d5f3c942bbfae31035057c2f.camel@codeconstruct.com.au>
Date: Fri, 18 Oct 2024 12:10:35 +0800
From: Matt Johnston <matt@...econstruct.com.au>
To: Kuniyuki Iwashima <kuniyu@...zon.com>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org, Roopa
Prabhu <roopa@...dia.com>, Nikolay Aleksandrov <razor@...ckwall.org>, David
Ahern <dsahern@...nel.org>, Jeremy Kerr <jk@...econstruct.com.au>
Subject: Re: [PATCH v2 net-next 13/14] rtnetlink: Return int from
rtnl_af_register().
On Wed, 2024-10-16 at 11:53 -0700, Kuniyuki Iwashima wrote:
> The next patch will add init_srcu_struct() in rtnl_af_register(),
> then we need to handle its error.
>
> Let's add the error handling in advance to make the following
> patch cleaner.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
The error cleanup changes look correct to me.
Reviewed-by: Matt Johnston <matt@...econstruct.com.au>
> ---
> Cc: Roopa Prabhu <roopa@...dia.com>
> Cc: Nikolay Aleksandrov <razor@...ckwall.org>
> Cc: David Ahern <dsahern@...nel.org>
> Cc: Jeremy Kerr <jk@...econstruct.com.au>
> Cc: Matt Johnston <matt@...econstruct.com.au>
> ---
> include/net/rtnetlink.h | 2 +-
> net/bridge/br_netlink.c | 6 +++++-
> net/core/rtnetlink.c | 4 +++-
> net/ipv4/devinet.c | 3 ++-
> net/ipv6/addrconf.c | 5 ++++-
> net/mctp/device.c | 16 +++++++++++-----
> net/mpls/af_mpls.c | 5 ++++-
> 7 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
> index 1a6aa5ca74f3..969138ae2f4b 100644
> --- a/include/net/rtnetlink.h
> +++ b/include/net/rtnetlink.h
> @@ -204,7 +204,7 @@ struct rtnl_af_ops {
> size_t (*get_stats_af_size)(const struct net_device *dev);
> };
>
> -void rtnl_af_register(struct rtnl_af_ops *ops);
> +int rtnl_af_register(struct rtnl_af_ops *ops);
> void rtnl_af_unregister(struct rtnl_af_ops *ops);
>
> struct net *rtnl_link_get_net(struct net *src_net, struct nlattr *tb[]);
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 6b97ae47f855..3e0f47203f2a 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -1924,7 +1924,9 @@ int __init br_netlink_init(void)
> if (err)
> goto out;
>
> - rtnl_af_register(&br_af_ops);
> + err = rtnl_af_register(&br_af_ops);
> + if (err)
> + goto out_vlan;
>
> err = rtnl_link_register(&br_link_ops);
> if (err)
> @@ -1934,6 +1936,8 @@ int __init br_netlink_init(void)
>
> out_af:
> rtnl_af_unregister(&br_af_ops);
> +out_vlan:
> + br_vlan_rtnl_uninit();
> out:
> return err;
> }
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 445e6ffed75e..70b663aca209 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -686,11 +686,13 @@ static const struct rtnl_af_ops *rtnl_af_lookup(const int family)
> *
> * Returns 0 on success or a negative error code.
> */
> -void rtnl_af_register(struct rtnl_af_ops *ops)
> +int rtnl_af_register(struct rtnl_af_ops *ops)
> {
> rtnl_lock();
> list_add_tail_rcu(&ops->list, &rtnl_af_ops);
> rtnl_unlock();
> +
> + return 0;
> }
> EXPORT_SYMBOL_GPL(rtnl_af_register);
>
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index d81fff93d208..2152d8cfa2dc 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -2812,7 +2812,8 @@ void __init devinet_init(void)
> register_pernet_subsys(&devinet_ops);
> register_netdevice_notifier(&ip_netdev_notifier);
>
> - rtnl_af_register(&inet_af_ops);
> + if (rtnl_af_register(&inet_af_ops))
> + panic("Unable to register inet_af_ops\n");
>
> rtnl_register_many(devinet_rtnl_msg_handlers);
> }
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index ac8645ad2537..d0a99710d65d 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -7468,7 +7468,9 @@ int __init addrconf_init(void)
>
> addrconf_verify(&init_net);
>
> - rtnl_af_register(&inet6_ops);
> + err = rtnl_af_register(&inet6_ops);
> + if (err)
> + goto erraf;
>
> err = rtnl_register_many(addrconf_rtnl_msg_handlers);
> if (err)
> @@ -7482,6 +7484,7 @@ int __init addrconf_init(void)
> errout:
> rtnl_unregister_all(PF_INET6);
> rtnl_af_unregister(&inet6_ops);
> +erraf:
> unregister_netdevice_notifier(&ipv6_dev_notf);
> errlo:
> destroy_workqueue(addrconf_wq);
> diff --git a/net/mctp/device.c b/net/mctp/device.c
> index 85cc5f31f1e7..3d75b919995d 100644
> --- a/net/mctp/device.c
> +++ b/net/mctp/device.c
> @@ -535,14 +535,20 @@ int __init mctp_device_init(void)
> int err;
>
> register_netdevice_notifier(&mctp_dev_nb);
> - rtnl_af_register(&mctp_af_ops);
> +
> + err = rtnl_af_register(&mctp_af_ops);
> + if (err)
> + goto err_notifier;
>
> err = rtnl_register_many(mctp_device_rtnl_msg_handlers);
> - if (err) {
> - rtnl_af_unregister(&mctp_af_ops);
> - unregister_netdevice_notifier(&mctp_dev_nb);
> - }
> + if (err)
> + goto err_af;
>
> + return 0;
> +err_af:
> + rtnl_af_unregister(&mctp_af_ops);
> +err_notifier:
> + unregister_netdevice_notifier(&mctp_dev_nb);
> return err;
> }
>
> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index a0573847bc55..1f63b32d76d6 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -2753,7 +2753,9 @@ static int __init mpls_init(void)
>
> dev_add_pack(&mpls_packet_type);
>
> - rtnl_af_register(&mpls_af_ops);
> + err = rtnl_af_register(&mpls_af_ops);
> + if (err)
> + goto out_unregister_dev_type;
>
> err = rtnl_register_many(mpls_rtnl_msg_handlers);
> if (err)
> @@ -2773,6 +2775,7 @@ static int __init mpls_init(void)
> rtnl_unregister_many(mpls_rtnl_msg_handlers);
> out_unregister_rtnl_af:
> rtnl_af_unregister(&mpls_af_ops);
> +out_unregister_dev_type:
> dev_remove_pack(&mpls_packet_type);
> out_unregister_pernet:
> unregister_pernet_subsys(&mpls_net_ops);
Powered by blists - more mailing lists