netdev - [PATCH] pppoe: fix race at init time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4A6F397D.6010606@gmail.com>
Date:	Tue, 28 Jul 2009 19:46:37 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	"David S. Miller" <davem@...emloft.net>
CC:	Pavel Emelyanov <xemul@...nvz.org>,
	Igor M Podlesny <for.poige+bugzilla.kernel.org@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	netdev@...r.kernel.org, Cyrill Gorcunov <gorcunov@...il.com>
Subject: [PATCH] pppoe: fix race at init time

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 <eric.dumazet@...il.com>
---
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;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html