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: w...@denx.de
"I haven't lost my mind - it's backed up on tape somewhere."
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
