From: Ido Schimmel <idosch@nvidia.com>
To: Minhong He <heminhong@kylinos.cn>
Cc: David Ahern <dsahern@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Simon Horman <horms@kernel.org>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net] ipv4: nexthop: handle errors in nexthop_init()
Date: Wed, 29 Jul 2026 14:10:50 +0300 [thread overview]
Message-ID: <20260729111050.GA1311000@shredder> (raw)
In-Reply-To: <20260728031055.76708-1-heminhong@kylinos.cn>
On Tue, Jul 28, 2026 at 11:10:55AM +0800, Minhong He wrote:
> nexthop_init() ignores errors from register_pernet_subsys(),
> register_netdevice_notifier() and rtnl_register_many(), so a partial
> initialization can appear successful.
>
> Check each step and unwind the registrations that already succeeded.
>
> Signed-off-by: Minhong He <heminhong@kylinos.cn>
> ---
> net/ipv4/nexthop.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index 6205bd57aa85..0ad40d93e3b5 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c
> @@ -4192,12 +4192,26 @@ static const struct rtnl_msg_handler nexthop_rtnl_msg_handlers[] __initconst = {
>
> static int __init nexthop_init(void)
> {
> - register_pernet_subsys(&nexthop_net_ops);
> + int err;
> +
> + err = register_pernet_subsys(&nexthop_net_ops);
> + if (err)
> + return err;
This can only fail if the allocation of devhash didn't succeed, which
seems unlikely (boot + GFP_KERNEL), but OK.
>
> - register_netdevice_notifier(&nh_netdev_notifier);
> + err = register_netdevice_notifier(&nh_netdev_notifier);
> + if (err)
> + goto err_unregister_pernet;
Can only fail if the notifier is already registered (not possible) or if
the NETDEV_{REGISTER,UP} replay fails which is also not possible given
that nh_netdev_event() always returns NOTIFY_DONE, but let's assume that
it's fragile and we can't rely on that.
>
> - rtnl_register_many(nexthop_rtnl_msg_handlers);
> + err = rtnl_register_many(nexthop_rtnl_msg_handlers);
> + if (err)
> + goto err_unregister_notifier;
Panics upon failure.
So, I think you should drop the error handling from rtnl_register_many()
(explain why it can't fail in the commit message) and target net-next
given the lack of fixes tag and that the issue is theoretical.
>
> return 0;
> +
> +err_unregister_notifier:
> + unregister_netdevice_notifier(&nh_netdev_notifier);
> +err_unregister_pernet:
> + unregister_pernet_subsys(&nexthop_net_ops);
> + return err;
> }
> subsys_initcall(nexthop_init);
> --
> 2.25.1
>
prev parent reply other threads:[~2026-07-29 11:11 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-28 3:10 [PATCH net] ipv4: nexthop: handle errors in nexthop_init() Minhong He
2026-07-29 11:10 ` Ido Schimmel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260729111050.GA1311000@shredder \
--to=idosch@nvidia.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=heminhong@kylinos.cn \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.