
Hello Nobuhiro,
Am 07.04.2016 um 22:31 schrieb 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.
Maybe you try the following patch on your hardware:
diff --git a/common/dlmalloc.c b/common/dlmalloc.c index b5bb051..cb2a5d8 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -2797,6 +2797,15 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes;
*/
+/* + * from: + * http://www.exploringbinary.com/ten-ways-to-check-if-an-integer-is-a-power-of... + * "10. Complement and Compare" + */ +static int isPowerOfTwo(size_t x) +{ + return ((x != 0) && ((x & (~x + 1)) == x)); +}
#if __STD_C Void_t* mEMALIGn(size_t alignment, size_t bytes) @@ -2824,6 +2833,12 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes;
if (alignment < MINSIZE) alignment = MINSIZE;
+ /* check if alignment is a power of 2 */ + if (isPowerOfTwo(alignment) == 0) { + printf("%s: %d is not power of 2!\n", __func__, alignment); + return NULL; + } + /* Call malloc with worst case padding to hit alignment. */
nb = request2size(bytes);
bye, Heiko