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

Submitted by:  Kieran Mansley <kieranm>
Submitted on:  Fri 17 Apr 2009 10:46:55 AM UTC  
 
Category: pbufsSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned 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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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:

    1. 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    2. 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>
Project Administrator

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 25 Apr 2009 05:44:25 PM UTCgoldsimonStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Sat 18 Apr 2009 11:24:07 AM UTCgoldsimonAssigned toNone=>goldsimon
      Planned Release=>1.3.1

    Back to the top


    Powered by Savane 3.1-cleanup1