[U-Boot] Potential memory corruption in drivers/net/sh_eth.c ?

[Wolfgang Denk]

Dear Nobuhiro,

while tracking down a memory corruption bug in other code, I ran over
these lines in  drivers/net/sh_eth.c :

...
194         /*
195          * Allocate rx descriptors. They must be aligned to size of struct
196          * tx_desc_s.
197          */
198         port_info->tx_desc_alloc =
199                 memalign(sizeof(struct tx_desc_s), alloc_desc_size);

...
246         /*
247          * Allocate rx descriptors. They must be aligned to size of struct
248          * rx_desc_s.
249          */
250         port_info->rx_desc_alloc =
251                 memalign(sizeof(struct rx_desc_s), alloc_desc_size);


There is some padding done (in drivers/net/sh_eth.h) to the stucts
tx_desc_s and rx_desc_s, but it appears onecritical fact is nowhere
checked:

Quoting from "common/dlmalloc.c":

....
2784   memalign algorithm:
2785
2786     memalign requests more than enough space from malloc, finds a spot
2787     within that chunk that meets the alignment request, and then
2788     possibly frees the leading and trailing space.
2789
2790     The alignment argument must be a power of two. This property is not
2791     checked by memalign, so misuse may result in random runtime errors.
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I. e. it is _mandatory_ that the first argument to memalign() must be
a power of two.  The current code does not guarantee this, and the
comments in the code (drivers/net/sh_eth.h) do not hint on this
restriction either:

...
 51 /* The size of the tx descriptor is determined by how much padding is used.
 52    4, 20, or 52 bytes of padding can be used */

I recommend to make this restriction more visible in the code and in
the comment, and/or even add a compile time test to guarantee this
requirement is met.


Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I haven't lost my mind - it's backed up on tape somewhere."