pppoe_init() ignores register_netdevice_notifier() errors and always
returns success after installing packet handlers, which can leave the
module loaded without its netdev notifier registered.

Check the error and unwind the packet handlers and protocol registration
on failure.

Signed-off-by: Minhong He <heminhong@kylinos.cn>
---
 drivers/net/ppp/pppoe.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 4a018acb5262..035795b120e0 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -1278,10 +1278,19 @@ static int __init pppoe_init(void)
 		dev_add_offload(&pppoe_packet_offload);
 	dev_add_pack(&pppoes_ptype);
 	dev_add_pack(&pppoed_ptype);
-	register_netdevice_notifier(&pppoe_notifier);
+
+	err = register_netdevice_notifier(&pppoe_notifier);
+	if (err)
+		goto out_unregister_packs;
 
 	return 0;
 
+out_unregister_packs:
+	dev_remove_pack(&pppoed_ptype);
+	dev_remove_pack(&pppoes_ptype);
+	if (IS_ENABLED(CONFIG_INET))
+		dev_remove_offload(&pppoe_packet_offload);
+	unregister_pppox_proto(PX_PROTO_OE);
 out_unregister_pppoe_proto:
 	proto_unregister(&pppoe_sk_proto);
 out_unregister_net_ops:
-- 
2.25.1

Hi,

On 2026/8/3 16:59, Minhong He wrote:
> pppoe_init() ignores register_netdevice_notifier() errors and always
> returns success after installing packet handlers, which can leave the
> module loaded without its netdev notifier registered.
>
> Check the error and unwind the packet handlers and protocol registration
> on failure.

A patch for the net tree requires a Fixes tag. As the notifier has been 
present since day 1 of the Linux git repository, you can use:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

> Signed-off-by: Minhong He <heminhong@kylinos.cn>
> ---
>   drivers/net/ppp/pppoe.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index 4a018acb5262..035795b120e0 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -1278,10 +1278,19 @@ static int __init pppoe_init(void)
>   		dev_add_offload(&pppoe_packet_offload);
>   	dev_add_pack(&pppoes_ptype);
>   	dev_add_pack(&pppoed_ptype);
> -	register_netdevice_notifier(&pppoe_notifier);
> +
> +	err = register_netdevice_notifier(&pppoe_notifier);
> +	if (err)
> +		goto out_unregister_packs;


You can move the registration above the dev_add_offload(), so that the 
unwind path is cleaner.


>   
>   	return 0;
>   
> +out_unregister_packs:
> +	dev_remove_pack(&pppoed_ptype);
> +	dev_remove_pack(&pppoes_ptype);
> +	if (IS_ENABLED(CONFIG_INET))
> +		dev_remove_offload(&pppoe_packet_offload);
> +	unregister_pppox_proto(PX_PROTO_OE);
>   out_unregister_pppoe_proto:
>   	proto_unregister(&pppoe_sk_proto);
>   out_unregister_net_ops:

Best regards,

Qingfang

On 8/4/2026 4:28 PM, Qingfang Deng wrote:
> On 2026/8/3 16:59, Minhong He wrote:
>> pppoe_init() ignores register_netdevice_notifier() errors and always
>> returns success after installing packet handlers, which can leave the
>> module loaded without its netdev notifier registered.
>>
>> Check the error and unwind the packet handlers and protocol registration
>> on failure.
> 
> A patch for the net tree requires a Fixes tag. As the notifier has been 
> present since day 1 of the Linux git repository, you can use:
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 
>> Signed-off-by: Minhong He <heminhong@kylinos.cn>
>> ---
>>   drivers/net/ppp/pppoe.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
>> index 4a018acb5262..035795b120e0 100644
>> --- a/drivers/net/ppp/pppoe.c
>> +++ b/drivers/net/ppp/pppoe.c
>> @@ -1278,10 +1278,19 @@ static int __init pppoe_init(void)
>>           dev_add_offload(&pppoe_packet_offload);
>>       dev_add_pack(&pppoes_ptype);
>>       dev_add_pack(&pppoed_ptype);
>> -    register_netdevice_notifier(&pppoe_notifier);
>> +
>> +    err = register_netdevice_notifier(&pppoe_notifier);
>> +    if (err)
>> +        goto out_unregister_packs;
> 
> You can move the registration above the dev_add_offload(), so that the 
> unwind path is cleaner.
> 

AI-review found a use-after-free. To avoid that, this needs to be placed 
between register_pernet_device() and proto_register().

>>       return 0;
>> +out_unregister_packs:
>> +    dev_remove_pack(&pppoed_ptype);
>> +    dev_remove_pack(&pppoes_ptype);
>> +    if (IS_ENABLED(CONFIG_INET))
>> +        dev_remove_offload(&pppoe_packet_offload);
>> +    unregister_pppox_proto(PX_PROTO_OE);
>>   out_unregister_pppoe_proto:
>>       proto_unregister(&pppoe_sk_proto);
>>   out_unregister_net_ops:

Best regards,

Qingfang

On Tue, 4 Aug 2026 22:20:53 +0800 Qingfang Deng wrote:
> >> +    err = register_netdevice_notifier(&pppoe_notifier);
> >> +    if (err)
> >> +        goto out_unregister_packs;  
> > 
> > You can move the registration above the dev_add_offload(), so that the 
> > unwind path is cleaner.
> >   
> 
> AI-review found a use-after-free. To avoid that, this needs to be placed 
> between register_pernet_device() and proto_register().

SG, but please note that register_netdevice_notifier()
cannot actually fail unless the notifier that's getting
registered fails.

Please do not add the Fixes tag, re-target this at net-next
and add something like:

   This is a future looking check, register_netdevice_notifier()
   only fails on double registration or if the registered notifier
   itself returns an error.

to the commit msg