[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[lwip-devel] [bug #27079] Yet another leak in PPP
From: |
Iordan Neshev |
Subject: |
[lwip-devel] [bug #27079] Yet another leak in PPP |
Date: |
Wed, 22 Jul 2009 15:46:46 +0000 |
User-agent: |
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.10) Gecko/2009042316 Firefox/3.0.10 |
URL:
<http://savannah.nongnu.org/bugs/?27079>
Summary: Yet another leak in PPP
Project: lwIP - A Lightweight TCP/IP stack
Submitted by: iordan_neshev
Submitted on: Wed 22 Jul 2009 03:46:45 PM GMT
Category: None
Severity: 3 - Normal
Item Group: None
Status: None
Privacy: Public
Assigned to: None
Open/Closed: Open
Discussion Lock: Any
Planned Release:
lwIP version: CVS Head
_______________________________________________________
Details:
In ppp.c, err_t pppInit(void):
Recently I posted a mail and filed a bug about a possible leak
in pppInit() caused by
outpacket_buf[i] = (u_char *)mem_malloc(PPP_MRU+PPP_HDRLEN);
My further investigation shows that outpacket_buf is never deallocated, at
least I could not find where. Correct me if I am wrong.
I beleive the right place to do this is just before the end of the ppp thread
function:
static void
pppMain(void *arg)
{
...
p = pbuf_alloc(PBUF_RAW, PPP_MRU+PPP_HDRLEN, PBUF_RAM);
if (!p) {
LWIP_ASSERT("p != NULL", p);
pc->errCode = PPPERR_ALLOC;
goto out;
}
...
while (lcp_phase[pd] != PHASE_DEAD) {
...
sys_msleep(1); /* give other tasks a chance to run */
...
}
PPPDEBUG((LOG_INFO, "pppMain: unit %d: PHASE_DEADn", pd));
pppDrop(pc); /* bug fix #17726 */
pbuf_free(p);
out:
PPPDEBUG(...);
if(pc->linkStatusCB) {
pc->linkStatusCB(pc->linkStatusCtx, pc->errCode ? pc->errCode :
PPPERR_PROTOCOL, NULL);
}
// ++++++++++++++++++++++ --->
LWIP_ASSERT("outpacket_buf is valid", (outpacket_buf[pd] != NULL));
//if (outpacket_buf[pd])// this should be here if I am not right about the
ASSERT.
mem_free(outpacket_buf[pd]);
// +++++++++++++++++++++++ <---
pc->openFlag = 0;
}
I was thinking about adding mem_free somewhere in the callback function
linkStatusCB,but it is not safe to do it because:
1. this may break someone's code, which assumes this buffer is valid and
2. this callback function is optional, the user has the freedom to define it
or not. When such function is not present (grep for if(!linkStatusCB) and
if(!pc->linkStatusCB)), usually (90%) the stack waits for (lcp_phase[pd] ==
PHASE_DEAD) and then calls pppClose(pd).
I thought about placing this mem_free in pppClose(), but then it may miss the
PPPoE case and, worse, it will not work if the first pbuf_alloc in pppMain()
fails.
I.e. this must be after the out: label.
Unfortunately, I can not test this since we do not use OS.
_______________________________________________________
Reply to this item at:
<http://savannah.nongnu.org/bugs/?27079>
_______________________________________________
Message sent via/by Savannah
http://savannah.nongnu.org/
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [lwip-devel] [bug #27079] Yet another leak in PPP,
Iordan Neshev <=