Subject: [PATCH] pppoe: fix race at init time

From: Eric Dumazet <[email-masked]>
Date: Tue, 28 Jul 2009 19:46:37 +0200

Eric Dumazet a écrit :
> Eric Dumazet a écrit :
>> Seems drivers/net/pppol2tp.c is a suspect...
>>
>> It uses register_pernet_gen_device() from pppol2tp_init()
>> but doesnt call unregister_pernet_gen_device()
> 
> OK patch seems really easy...
> 
> This bug was added in commit 4e9fb8016a351b5b9da7fea32bcfdbc9d836e421
> net: pppol2tp - introduce net-namespace functionality
> 
> So this is a stable candidate I guess ?
> 
> Thank you

So Igor still has a panic... lets try a third patch then :)

[PATCH] pppoe: fix race at init time

I believe we have a race in ppoe_init() :

As soon as dev_add_pack(&pppoes_ptype); and/or dev_add_pack(&pppoed_ptype); 
are called, we can receive packets while nets not yet fully ready
(ie : pppoe_init_net() not yet called)

This means we should be prepared to get a NULL pointer
from net_generic(net, pppoe_net_id) call.

We miss this NULL check in get_item() and possibly crash if this nets 
has no struct pppoe_net attached yet. Other subroutines
are safe.

Signed-off-by: Eric Dumazet <[email-masked]>
---
diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
index f0031f1..e50af8c 100644
--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -237,14 +237,15 @@ static struct pppox_sock *__delete_item(struct pppoe_net *pn, __be16 sid,
 static inline struct pppox_sock *get_item(struct pppoe_net *pn, __be16 sid,
 					unsigned char *addr, int ifindex)
 {
-	struct pppox_sock *po;
-
-	read_lock_bh(&pn->hash_lock);
-	po = __get_item(pn, sid, addr, ifindex);
-	if (po)
-		sock_hold(sk_pppox(po));
-	read_unlock_bh(&pn->hash_lock);
-
+	struct pppox_sock *po = NULL;
+
+	if (pn) {
+		read_lock_bh(&pn->hash_lock);
+		po = __get_item(pn, sid, addr, ifindex);
+		if (po)
+			sock_hold(sk_pppox(po));
+		read_unlock_bh(&pn->hash_lock);
+	}
 	return po;
 }