buglwIP - A Lightweight TCP/IP stack - Bugs: bug #26213, Problem with memory allocation...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #26213: Problem with memory allocation when debugging

Submitter:  Kieran Mansley <kieranm>
Submitted:  Fri 17 Apr 2009 10:46:55 AM UTC
   
 
Category:  pbufs Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  1.3.0

Sat 25 Apr 2009 05:44:25 PM UTC, comment #3: 

Fixed by fixing the sizes stored in memp_sizes. I also made the malloc-pools a little bigger (so that the sizes define in lwippools.h are what the user can malloc, not what is reserved) and added a new configuration option to try the next bigger pool if one is empty (removes a @todo in mem.c).

Simon Goldschmidt <goldsimon>
Group administrator
Mon 20 Apr 2009 06:42:19 PM UTC, comment #2: 

Actually, I just discovered this is a bug in the calculation of memp_sizes with MEMP_OVERFLOW_CHECK/MEMP_SANITY_CHECK turned on. I'm on it...

Simon Goldschmidt <goldsimon>
Group administrator
Sat 18 Apr 2009 11:24:07 AM UTC, comment #1: 

I'm glad you added this, I've completely forgotton about it. Seems that's my fault, so I hope to get the time to look through this.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 17 Apr 2009 10:46:55 AM UTC, original submission:  

Marek MaƂowidzki has reported a problem on lwip-devel:

I believe that there is a problem with memory allocation when memory allocation debugging is on (MEMP_OVERFLOW_CHECK is defined as 2 and MEMP_SANITY_CHECK as 1). In etharp_raw() in etharp.c, a call to
 
p = pbuf_alloc(PBUF_LINK, sizeof(struct etharp_hdr), PBUF_RAM);

causes the following buffer to be returned:

00 00 00 00  00 00 00 00  00 00 00 00  00 00 00 00
00 00 00 00  00 00 00 00  00 00 00 00  cd cd cd cd
cd cd cd cd  cd cd cd cd  cd cd cd cd

(0xcd chars are memory guards) and when we call

memset(p->payload, 0, p->len);

which should always be correct, a subsequent call to memp_overflow_check_all() shows that the guards have been overwritten.

A likely cause is a bug in mem_malloc(): the size of a pool selected for allocation grows from 64 (for MEMP_POOL_64 and no memory debugging) to 80, i.e., it looks like the size is increased by the size of the guards and a too-large memory segment is allocated. When we change PBUF_LINK to PBUF_RAW, no problem appears in this place (btw, shouldn't this really be PBUF_RAW here?).



Ok, so I think I have found the problem (1.3.0).
 
memp_sizes is defined in memp.c as:
 
const u16_t memp_sizes[MEMP_MAX] = {

#define LWIP_MEMPOOL(name,num,size,desc) MEMP_ALIGN_SIZE(size),

#include "lwip/memp_std.h"

};

where

#define MEMP_ALIGN_SIZE(x) (LWIP_MEM_ALIGN_SIZE(x) + MEMP_SANITY_REGION_AFTER_ALIGNED)

so, the block size is increased by the sanity region length.

Now - in mem_malloc() (mem.c), the expression that looks for a sufficiently large block of memory is as follows:
 
if ((size + sizeof(struct mem_helper)) <= memp_sizes[poolnr]) {

however, memp_sizes have been increased; thus, the selected block may be too small and writing the whole allocated region will overwrite the guards.

I think that, due to code organization, which is quite complex, it is difficult for me to suggest a neat patch. For now, I have introduced another array,

const u16_t memp_bare_sizes[MEMP_MAX] = {

#define LWIP_MEMPOOL(name,num,size,desc) LWIP_MEMP_ALIGN_SIZE(size),

#include "lwip/memp_std.h"

};

and changed mem_malloc() to
 
if ((size + sizeof(struct mem_helper)) <= memp_bare_sizes[poolnr]) {
 
the problem seems to be solved.
 
Also, as I suggested previously, it seems that etharp_raw() should change PBUF_LINK to PBUF_RAW in the following line:
 
p = pbuf_alloc(PBUF_LINK, sizeof(struct etharp_hdr), PBUF_RAM);
 
since it creates the whole Ethernet frame at once, no additional space for an Ethernet header is necessary at the beginning.

Kieran Mansley <kieranm>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kieranm (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-04-25 goldsimon StatusNone Fixed
        Open/ClosedOpen Closed
    2009-04-18 goldsimon Assigned toNone goldsimon
        Planned Release 1.3.1

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code