Hi guys,
Short version: Any use of ppp can lead to memory corruption. Patch below.
We've been trying to track an odd memory corruption problem for a while,
and someone recently managed to narrow it down to PPPoE. I kinda drew the
short straw, as my laptop was then able to relaibly reproduce.
Anyway, the bug is that ppp_generic.c has two places that kfree the ppp
struct, and not enough locking to make sure they don't both free it if
ppp_destroy_interface schedules while unregistering the device.
This patch also has a few hunks for pppoe.c, setting a few variables to 0
to make sure they don't get freed twice.
A larger problem is that ppp_generic.c has numerous schedules inside
spinlocks (including the one that caused the double free ;-). Kurt Garloff
supplied some hunks to swtich GFP_KERNEL to GFP_ATOMIC, but I'm hoping
someone who knows the code is interested in fixing the rest of the spin
lock abuses. Otherwise I might try to tackle it next week.
Patch below, against 2.4.10. Please apply.
--- 3.1/drivers/net/pppoe.c Sun, 23 Sep 2001 16:21:54 -0400
+++ 3.1(w)/drivers/net/pppoe.c Wed, 26 Sep 2001 18:01:28 -0400
@@ -541,11 +541,15 @@
sk->state = PPPOX_DEAD;
po = sk->protinfo.pppox;
- if (po->pppoe_pa.sid)
+ if (po->pppoe_pa.sid) {
delete_item(po->pppoe_pa.sid, po->pppoe_pa.remote);
+ po->pppoe_pa.sid = 0 ;
+ }
if (po->pppoe_dev)
dev_put(po->pppoe_dev);
+
+ po->pppoe_dev = NULL ;
sock_orphan(sk);
sock->sk = NULL;
--- 3.1/drivers/net/ppp_generic.c Sun, 23 Sep 2001 20:11:16 -0400
+++ 3.1(w)/drivers/net/ppp_generic.c Thu, 27 Sep 2001 09:28:51 -0400
@@ -2105,13 +2108,12 @@
{
struct compressor_entry *ce;
int ret;
-
spin_lock(&compressor_list_lock);
ret = -EEXIST;
if (find_comp_entry(cp->compress_proto) != 0)
goto out;
ret = -ENOMEM;
- ce = kmalloc(sizeof(struct compressor_entry), GFP_KERNEL);
+ ce = kmalloc(sizeof(struct compressor_entry), GFP_ATOMIC);
if (ce == 0)
goto out;
ret = 0;
@@ -2216,11 +2218,11 @@
/* Create a new ppp structure and link it before `list'. */
ret = -ENOMEM;
- ppp = kmalloc(sizeof(struct ppp), GFP_KERNEL);
+ ppp = kmalloc(sizeof(struct ppp), GFP_ATOMIC);
if (ppp == 0)
goto out;
memset(ppp, 0, sizeof(struct ppp));
- dev = kmalloc(sizeof(struct net_device), GFP_KERNEL);
+ dev = kmalloc(sizeof(struct net_device), GFP_ATOMIC);
if (dev == 0) {
kfree(ppp);
goto out;
@@ -2285,6 +2287,7 @@
static void ppp_destroy_interface(struct ppp *ppp)
{
struct net_device *dev;
+ int n_channels ;
spin_lock(&all_ppp_lock);
list_del(&ppp->file.list);
@@ -2314,6 +2317,7 @@
#endif /* CONFIG_PPP_FILTER */
dev = ppp->dev;
ppp->dev = 0;
+ n_channels = ppp->n_channels ;
ppp_unlock(ppp);
if (dev) {
@@ -2329,7 +2333,7 @@
* ppp structure. Otherwise we leave it around until the
* last channel disconnects from it.
*/
- if (ppp->n_channels == 0)
+ if (n_channels == 0)
kfree(ppp);
spin_unlock(&all_ppp_lock);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sun Sep 30 2001 - 21:00:55 EST