[lwip-users] repost: MEM_ALIGN and pbuf.c

[Bonny Gijzen]

Hi guys,

 

 

It seems the bug which John Recker posted still exists in the latest lwip release (1.1.0 with some small updates from CVS), so I am reposting it again now to give it some more attention.

(A friend of mine bumped into this bug on his MIPS implementation)

 

Here is the original message from John Recker:

 

It appears to me that there are several problems
with the use of MEM_ALIGN in pbuf.c (the code is
from the 5.3 release, but appears unchanged in the
latest release)

Two that I noticed are:

pbuf_alloc(). Line 235..

    /* Set the payload pointer so that it points offset bytes into
       pbuf data memory. */
    p->payload = MEM_ALIGN((void *)((u8_t *)p + (sizeof(struct pbuf) +
offset)));

    /* The total length of the pbuf is the requested size. */
    p->tot_len = size;

    /* Set the length of the first pbuf is the chain. */
    p->len = size > PBUF_POOL_BUFSIZE - offset? PBUF_POOL_BUFSIZE - offset:
size;

The problem here is that p->len is not correct
if (p->payload != p). The size calculation should
also account for incrementing p->payload
for alignment.


pbuf_alloc(). Line 262

      q->payload = (void *)((u8_t *)q + sizeof(struct pbuf));

This should be wrapped with the MEM_ALIGN macro.

I think that there are additional problems.
But, at this point I started looking at the
base cause of the memory alignment problem.
pbuf_pool_memory itself is guaranteed to be
memory aligned. So, as long as PBUF_POOL_BUFSIZE
and sizeof(struct pbuf) are multiples of the
memory alignment, then so should any address
returned by pbuf_pool_alloc.

So I was able to work around the problem by
changing the definition of struct pbuf (note
that I am forcing byte alignment in all my
structs due to requirements of another library
I am using). True, this change adds 2 bytes/struct,
but the change does provide up to 64 bit alignment
if it is packed (and the code won't work on
machines w/ memory alignment requirements w/o
the extra 2 bytes anyways...)

struct pbuf {
  struct pbuf *next;
 
  void *payload;

  /* high 4 bits, flags, low 4 bits reference count */
  u16_t flags, ref;
 
  /* Total length of buffer + additionally chained buffers. */
  u16_t tot_len;
  /* Length of this buffer. */
  u16_t len; 
 
};