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

[Iordan Neshev]

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);
   }
 }
// ------------------------------------------- 

Should I file a bug about this?

//   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.

I'm concerned about freeing outpacket_buf[] later if the
link drops. I'm sniffing a leak there too. I'll investigate this more.