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

Qingfang Deng <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.