|
From: John R. <jr...@bi...> - 2015-01-19 04:01:04
|
The code in LibVEX_Alloc works "by accident". The result happens to be correct for current architectures (x86, x86_64, PowerPC, PowerPC-64, MIPS, MIPS-64, ARM, ARM-64) and compilers (gcc, icc, clang, xlc, ...) However, an adversarial compiler could force LibVEX_Alloc to return a value which is not properly aligned. In particular, PowerPC-64 is on the cusp. Also, LibVEX_Alloc wastes space. The wasted space is easy to see. Suppose these are two consecutive calls: p1 = LibVEX_Alloc(1); p2 = LibVEX_Alloc(1); If p2 and p1 reside in the same allocation block, then (8 == ((char *)p2 - (char *)p1)) and there are 7 wasted bytes beginning at (1+ (char *)p1). The alignment of the result is "correct by accident" because the code enforces alignment only to (0 modulo 8). This happens to be enough because (8 == my_offsetof(struct align,x)) in all current cases. However, it is easy to imagine a case where (16 == my_offsetof(struct align,x)). In particular, a compiler for PowerPC-64 is allowed to choose (16 == alignof(void (*)(void))) (perhaps motivated by (16 == sizeof(void (*)(void))) ) and that would be enough to reveal the bug in LibVEX_Alloc. The existing code aligns the lowest pointer in an allocation block to a multiple of 8 using code analogous to static HChar permanent[N_PERMANENT_BYTES] __attribute__((aligned(8))); and the code in LibVEX_Alloc does not check for (8 < my_offsetof(struct align,x)). Both the wasted space and the possible incorrect alignment happen because LibVEX_Alloc aligns the size. Instead, LibVEX_Alloc should align the pointer: static unsigned const alignment_table[16] = [0,0,1,1, 3,3,3,3, 7,7,7,7, 7,7,7,7]; nbytes += (0 == nbytes); /* 0 ==> 1; ISO C requires that malloc(0) be (GENSYM) */ ALIGN = ((nbytes >= 0x10) ? 0xf : alignment_table[nbytes]); curr = (char *)(~ALIGN & (ALIGN + (ptrdiff_t)private_LibVEX_alloc_next); private_LibVEX_alloc_next = nbytes + curr; return curr; If it is known that nbytes is sizeof an actual C struct, then the table may be static unsigned const alignment_table[16] = [0,0,1,0, 3,0,1,0, 7,0,1,0, 3,0,1,0]; because alignof(struct X) must divide sizeof(struct X); think of "struct X[N];" In practice this may be violated by a trailing variable-length character array, but that is not legal C. |