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