memory_buffer_alloc should use a sufficient default alignment
- Dominant language
- C
- Stars
- 108
- Forks
- 95
- PR merge metrics
- No merged PRs in 30d
Description
### Description
The MEMORY_BUFFER_ALLOC module defaults to aligning blocks on a 4-byte boundary.
```
include/mbedtls/memory_buffer_alloc.h:#define MBEDTLS_MEMORY_ALIGN_MULTIPLE 4 /**< Align on multiples of this value */
```
On architectures that require 8- or 16-byte alignment for some data, this results in buggy code.
The fix is to default `MBEDTLS_MEMORY_ALIGN_MULTIPLE` to something sensible. In C11, that's `_Alignof(max_align_t)`. In older C dialects, should we use `sizeof(char*)` (backward compatible) or `2 * sizeof(char*)` (I don't know of a machine where it's required, but it can improve performance with vector instructions)?
To test, we should run a job in `all.sh` with `MBEDTLS_MEMORY_BUFFER_ALLOC_C` enabled and UBSan enabled (it detects misaligned pointer accesses). At the moment, `MBEDTLS_MEMORY_BUFFER_ALLOC_C` is tested via `config.pl` full. In Mbed TLS, we've changed that to a dedicated `component_test_memory_buffer_allocator`. We should wait until this change is sideported to crypto, then modify the test component to enable UBSan.
To reproduce:
```
scripts/config.pl set MBEDTLS_MEMORY_BUFFER_ALLOC_C
scripts/config.pl set MBEDTLS_MEMORY_DEBUG
scripts/config.pl set MBEDTLS_PLATFORM_MEMORY
make CFLAGS='-fsanitize=undefined' LDFLAGS='-fsanitize=undefined' lib tests
make test
```
### Issue request type
[ ] Question
[ ] Enhancement
[x] Bug
Contributor guide
Research direction
Development has moved to Mbed TLS, so first locate the corresponding memory buffer allocator there. Review include/mbedtls/memory_buffer_alloc.h and the scripts/config.pl reproduction, then inspect all.sh and component_test_memory_buffer_allocator; done means a suitable default alignment and UBSan coverage for the allocator.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- backend, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100