[lwip-devel] [bug #27078] Possible memory leak in pppInit()

[Iordan Neshev]

URL:
  <http://savannah.nongnu.org/bugs/?27078>

                 Summary:  Possible memory leak in pppInit()
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: iordan_neshev
            Submitted on: Wed 22 Jul 2009 03:42:57 PM GMT
                Category: None
                Severity: 3 - Normal
              Item Group: Change Request
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: 
            lwIP version: CVS Head

    _______________________________________________________

Details:

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.




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27078>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/