From e2d1f43bc10172272a74e2bcf2460b0dbc91aeea Mon Sep 17 00:00:00 2001 From: David Robillard Date: Tue, 17 May 2016 11:58:47 -0400 Subject: Fix unaligned memory access (UB which breaks ARM) With this fix, the test suite runs cleanly with UBSan. --- NEWS | 3 ++- src/reader.c | 12 ++++++------ src/serd_internal.h | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/NEWS b/NEWS index 556abba9..e85744a8 100644 --- a/NEWS +++ b/NEWS @@ -1,10 +1,11 @@ serd (0.22.3) unstable; * Fix potential out of bounds read + * Fix unaligned memory access, undefined behaviour which breaks on ARM * Fix documentation generation * Update serdi man page - -- David Robillard Tue, 15 Mar 2016 17:46:00 -0400 + -- David Robillard Tue, 17 May 2016 11:58:20 -0400 serd (0.22.0) stable; diff --git a/src/reader.c b/src/reader.c index e49c2a10..e6af0b1c 100644 --- a/src/reader.c +++ b/src/reader.c @@ -1,5 +1,5 @@ /* - Copyright 2011-2015 David Robillard + Copyright 2011-2016 David Robillard Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted, provided that the above @@ -173,8 +173,8 @@ static Ref push_node_padded(SerdReader* reader, size_t maxlen, SerdType type, const char* str, size_t n_bytes) { - uint8_t* mem = serd_stack_push(&reader->stack, - sizeof(SerdNode) + maxlen + 1); + void* mem = serd_stack_push_aligned( + &reader->stack, sizeof(SerdNode) + maxlen + 1, sizeof(SerdNode)); SerdNode* const node = (SerdNode*)mem; node->n_bytes = node->n_chars = n_bytes; @@ -182,13 +182,13 @@ push_node_padded(SerdReader* reader, size_t maxlen, node->type = type; node->buf = NULL; - uint8_t* buf = mem + sizeof(SerdNode); + uint8_t* buf = (uint8_t*)(node + 1); memcpy(buf, str, n_bytes + 1); #ifdef SERD_STACK_CHECK reader->allocs = realloc( reader->allocs, sizeof(uint8_t*) * (++reader->n_allocs)); - reader->allocs[reader->n_allocs - 1] = (mem - reader->stack.buf); + reader->allocs[reader->n_allocs - 1] = ((uint8_t*)mem - reader->stack.buf); #endif return (uint8_t*)node - reader->stack.buf; } @@ -243,7 +243,7 @@ pop_node(SerdReader* reader, Ref ref) #endif SerdNode* const node = deref(reader, ref); uint8_t* const top = reader->stack.buf + reader->stack.size; - serd_stack_pop(&reader->stack, top - (uint8_t*)node); + serd_stack_pop_aligned(&reader->stack, top - (uint8_t*)node); } return 0; } diff --git a/src/serd_internal.h b/src/serd_internal.h index 20700bb8..23fd75b4 100644 --- a/src/serd_internal.h +++ b/src/serd_internal.h @@ -1,5 +1,5 @@ /* - Copyright 2011-2014 David Robillard + Copyright 2011-2016 David Robillard Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted, provided that the above @@ -123,6 +123,38 @@ serd_stack_pop(SerdStack* stack, size_t n_bytes) stack->size -= n_bytes; } +static inline void* +serd_stack_push_aligned(SerdStack* stack, size_t n_bytes, size_t align) +{ + // Push one byte to ensure space for a pad count + serd_stack_push(stack, 1); + + // Push padding if necessary + const uint8_t pad = align - stack->size % align; + if (pad > 0) { + serd_stack_push(stack, pad); + } + + // Set top of stack to pad count so we can properly pop later + stack->buf[stack->size - 1] = pad; + + // Push requested space at aligned location + return serd_stack_push(stack, n_bytes); +} + +static inline void +serd_stack_pop_aligned(SerdStack* stack, size_t n_bytes) +{ + // Pop requested space down to aligned location + serd_stack_pop(stack, n_bytes); + + // Get amount of padding from top of stack + const uint8_t pad = stack->buf[stack->size - 1]; + + // Pop padding and pad count + serd_stack_pop(stack, pad + 1); +} + /* Bulk Sink */ typedef struct SerdBulkSinkImpl { -- cgit v1.2.1