The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net] net: pppoe: check register_netdevice_notifier() error in pppoe_init()
@ 2026-08-03  8:59 Minhong He
  2026-08-04  8:28 ` Qingfang Deng
  0 siblings, 1 reply; 4+ messages in thread
From: Minhong He @ 2026-08-03  8:59 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev
  Cc: Qingfang Deng, Kees Cook, Eric Woudstra, Felix Fietkau,
	linux-kernel

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


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

* Re: [PATCH net] net: pppoe: check register_netdevice_notifier() error in pppoe_init()
  2026-08-03  8:59 [PATCH net] net: pppoe: check register_netdevice_notifier() error in pppoe_init() Minhong He
@ 2026-08-04  8:28 ` Qingfang Deng
  2026-08-04 14:20   ` Qingfang Deng
  0 siblings, 1 reply; 4+ messages in thread
From: Qingfang Deng @ 2026-08-04  8:28 UTC (permalink / raw)
  To: Minhong He, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev
  Cc: Kees Cook, Eric Woudstra, Felix Fietkau, linux-kernel

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


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

* Re: [PATCH net] net: pppoe: check register_netdevice_notifier() error in pppoe_init()
  2026-08-04  8:28 ` Qingfang Deng
@ 2026-08-04 14:20   ` Qingfang Deng
  2026-08-05  1:52     ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: Qingfang Deng @ 2026-08-04 14:20 UTC (permalink / raw)
  To: Minhong He, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev
  Cc: Kees Cook, Eric Woudstra, Felix Fietkau, linux-kernel

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

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

* Re: [PATCH net] net: pppoe: check register_netdevice_notifier() error in pppoe_init()
  2026-08-04 14:20   ` Qingfang Deng
@ 2026-08-05  1:52     ` Jakub Kicinski
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-08-05  1:52 UTC (permalink / raw)
  To: Qingfang Deng
  Cc: Minhong He, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, netdev, Kees Cook, Eric Woudstra, Felix Fietkau,
	linux-kernel

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

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

end of thread, other threads:[~2026-08-05  1:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-03  8:59 [PATCH net] net: pppoe: check register_netdevice_notifier() error in pppoe_init() Minhong He
2026-08-04  8:28 ` Qingfang Deng
2026-08-04 14:20   ` Qingfang Deng
2026-08-05  1:52     ` Jakub Kicinski

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