From 04ecb267c0e1317849eab4b1c7e456c8a6e1fc7a Mon Sep 17 00:00:00 2001 From: Jessica Clarke Date: Tue, 4 Jan 2022 17:06:14 +0000 Subject: [PATCH] match: Properly align heapframes for CHERI/Arm's Morello prototype (#72) On CHERI, and thus Arm's Morello prototype, pointers are represented as hardware capabilities, which consist of both an integer address and additional metadata, meaning they are twice the size of the platform's size_t type, i.e. 16 bytes on a 64-bit system. The ovector member of heapframe happens to only be 8 byte aligned, and so computing frame_size ends up with a multiple of 8 but not 16. Whilst the first frame is always suitably aligned, this then misaligns the frame that follows it, resulting in an alignment fault when storing a pointer to Fecode at the start of match. Thus, round up frame_size to a multiple of heapframe's alignment to ensure alignment is preserved. This can be completely optimised away on traditional architectures and, since CHERI's capabilities are in fact 2 * sizeof(PCRE2_SIZE) bytes in size, the variable part of the expression is also proven to be a multiple of the alignment and so the aligning gets folded into the offsetof part by adding an additional 8, so no dynamic alignment code is needed even on CHERI architectures. --- src/pcre2_intmodedep.h | 11 +++++++++++ src/pcre2_match.c | 14 ++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/pcre2_intmodedep.h b/src/pcre2_intmodedep.h index 3daf21b..f8a3d25 100644 --- a/src/pcre2_intmodedep.h +++ b/src/pcre2_intmodedep.h @@ -838,6 +838,17 @@ multiple of PCRE2_SIZE. See various comments above. */ typedef char check_heapframe_size[ ((sizeof(heapframe) % sizeof(PCRE2_SIZE)) == 0)? (+1):(-1)]; +/* Structure for computing the alignment of heapframe. */ + +typedef struct heapframe_align { + char unalign; /* Completely unalign the current offset */ + heapframe frame; /* Offset is its alignment */ +} heapframe_align; + +/* This define is the minimum alignment required for a heapframe, in bytes. */ + +#define HEAPFRAME_ALIGNMENT offsetof(heapframe_align, frame) + /* Structure for passing "static" information around between the functions doing traditional NFA matching (pcre2_match() and friends). */ diff --git a/src/pcre2_match.c b/src/pcre2_match.c index df91fa2..24706e5 100644 --- a/src/pcre2_match.c +++ b/src/pcre2_match.c @@ -6781,10 +6781,16 @@ the pattern. It is not used at all if there are no capturing parentheses. The last of these is changed within the match() function if the frame vector has to be expanded. We therefore put it into the match block so that it is -correct when calling match() more than once for non-anchored patterns. */ +correct when calling match() more than once for non-anchored patterns. -frame_size = offsetof(heapframe, ovector) + - re->top_bracket * 2 * sizeof(PCRE2_SIZE); +We must also pad frame_size for alignment to ensure subsequent frames are as +aligned as heapframe. Whilst ovector is word-aligned due to being a PCRE2_SIZE +array, that does not guarantee it is suitably aligned for pointers, as some +architectures have pointers that are larger than a size_t. */ + +frame_size = (offsetof(heapframe, ovector) + + re->top_bracket * 2 * sizeof(PCRE2_SIZE) + HEAPFRAME_ALIGNMENT - 1) & + ~(HEAPFRAME_ALIGNMENT - 1); /* Limits set in the pattern override the match context only if they are smaller. */ @@ -6828,7 +6834,7 @@ mb->match_frames_top = to avoid uninitialized memory read errors when it is copied to a new frame. */ memset((char *)(mb->match_frames) + offsetof(heapframe, ovector), 0xff, - re->top_bracket * 2 * sizeof(PCRE2_SIZE)); + frame_size - offsetof(heapframe, ovector)); /* Pointers to the individual character tables */