Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memory_buffer_alloc should use a sufficient default alignment #311

Open
gilles-peskine-arm opened this issue Oct 31, 2019 · 1 comment
Open

Comments

@gilles-peskine-arm
Copy link
Collaborator

gilles-peskine-arm commented Oct 31, 2019

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
@ciarmcom
Copy link
Member

Internal Jira reference: https://jira.arm.com/browse/IOTCRYPT-967

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants