ext/hash relies on implementation-defined malloc alignment

[arnaud-lb]

Description

ext/hash uses ecalloc() to allocate context buffers in php_hash_alloc_context(), and assumes that the buffer will be 16 bytes aligned. If it's not, it crashes at least here:

php-src/ext/hash/xxhash/xxhash.h

Lines 4050 to 4054 in 0943b8b

	XXH_ASSERT(((size_t)src16 & 15) == 0); /* control alignment */
	XXH_ASSERT(((size_t)dst16 & 15) == 0);
	for (i=0; i < nbRounds; ++i) {
	dst16[i] = _mm_add_epi64(_mm_load_si128((const __m128i *)src16+i), seed);

Posix specifies that "The pointer returned [by calloc()] shall be suitably aligned so that it may be assigned to a pointer to any type of object and then used to access such an object or an array of such objects in the space allocated [...]".

On x86_64 there is no type requiring an alignment larger than 8 bytes, so it's really an implementation details that [e]calloc() returns a 16 bytes aligned buffer in php_hash_alloc_context(), and it's dependent on the requested size and the state of the heap.

The crash can be reproduced with the help of #18172:

ZEND_MM_DEBUG=padding=8 TESTS=ext/hash make test

PHP Version

PHP8.3

Operating System

No response

[arnaud-lb]

Actually, the max alignment on x86_64 is 16, not 8, so ext/hash's assumptions may be ok.

[ndossche]

php-src/ext/hash/xxhash/xxhash.h

Lines 1059 to 1062 in 0943b8b

	* @note ** This structure has a strict alignment requirement of 64 bytes!! **
	* Do not allocate this with `malloc()` or `new`,
	* it will not be sufficiently aligned.
	* Use @ref XXH3_createState() and @ref XXH3_freeState(), or stack allocation.

and indeed, that create function calls a function that allocates and aligns the allocation address:

php-src/ext/hash/xxhash/xxhash.h

Line 4645 in 0943b8b

static void* XXH_alignedMalloc(size_t s, size_t align)

It all depends on ecallocs guarantees