Re: [PATCH net] net: pppoe: check register_netdevice_notifier() error in pppoe_init()

Qingfang Deng <[email protected]> Tue, 4 Aug 2026 22:20:53 +0800
Newsgroups gmane.linux.network,gmane.linux.kernel
Message-ID <[email protected]>
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 <[email protected]>
>> ---
>>   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