Wed 22 Jul 2009 03:42:57 PM UTC, original submission:
In ppp.c:
pppInit() tries to allocate (PPP_MRU+PPP_HDRLEN) bytes for every ppp interface. If the interface is only one there is no problem. But if they are, let's say 3, consider this situation:
outpacket_buf[0] - allocated,
outpacket_buf[1] - allocated,
outpacket_buf[2] - not allocated.
Then pppInit() returns ERR_MEM, leaving 2 buffers allocated.
// -------------------------------------------
int i, j;
...
for (i = 0; i < NUM_PPP; i++) {
pppControl[i].openFlag = 0;
subnetMask = htonl(0xffffff00); // why is this here?
outpacket_buf[i] = (u_char *)mem_malloc(PPP_MRU+PPP_HDRLEN);
if(!outpacket_buf[i]) {
return ERR_MEM;
}
/*
* Initialize to the standard option set.
*/
for (j = 0; (protp = ppp_protocols[j]) != NULL; ++j) {
(*protp->init)(i);
}
}
// -------------------------------------------
I propose to change it like this:
// -------------------------------------------
subnetMask = htonl(0xffffff00);
for (i = 0; i < NUM_PPP; i++) {
pppControl[i].openFlag = 0;
outpacket_buf[i] = (u_char *)mem_malloc(PPP_MRU+PPP_HDRLEN);
if (!outpacket_buf[i]) {
// +++ -->
for (j = 0; j < i; j++) {
/* deallocate all preceding buffers */
mem_free(outpacket_buf[j]);
}
// +++ <--
return ERR_MEM;
}
/*
* Initialize to the standard option set.
*/
for (j = 0; (protp = ppp_protocols[j]) != NULL; ++j) {
(*protp->init)(i);
}
}
// -------------------------------------------
// offtopic:
// subnetMask should not be initialized a couple of times
// in the loop, once is enough.That's why I pulled it out.
// What do you think?
I doubt that anybody uses more than one ppp interface, so this
is a minor problem.
Since pppInit() is called by the user, we should leave a note
about this change because this may break someone's code.
|