From d01d23374107f6fc55511f02559cf75be7bdf448 Mon Sep 17 00:00:00 2001 From: Alex Richardson Date: Wed, 16 Jun 2021 12:17:04 +0100 Subject: [PATCH] Avoid undefined behaviour after realloc() Adding the offset between the realloc result and the old allocation to update pointers into the new allocation is undefined behaviour: the old pointers are no longer valid after realloc() according to the C standard. While this works on almost all architectures and compilers, it causes problems on architectures that track pointer bounds (e.g. CHERI or Arm's Morello): the value_list pointers will still have the bounds of the previous allocation and therefore any dereference will result in a run-time trap. I found this due to a crash (dereferencing an invalid capability) while trying to run `xev` over SSH on a CHERI-RISC-V system. With these two realloc changes, and https://gitlab.freedesktop.org/xorg/proto/xorgproto/-/merge_requests/41 I am able to succesfully run `xev` compiled for CHERI-RISC-V. Signed-off-by: Alex Richardson --- modules/im/ximcp/imLcPrs.c | 4 +++- src/xlibi18n/lcDB.c | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/modules/im/ximcp/imLcPrs.c b/modules/im/ximcp/imLcPrs.c index 60215c70..795ea719 100644 --- a/modules/im/ximcp/imLcPrs.c +++ b/modules/im/ximcp/imLcPrs.c @@ -676,8 +676,10 @@ parseline( goto error; b->tree = new; b->treesize = newsize; + /* Re-derive top after realloc() to avoid undefined behaviour + (and crashes on architectures that track pointer bounds). */ if (top >= (DTIndex *) old && top < (DTIndex *) &old[oldsize]) - top = (DTIndex *) (((char *) top) + (((char *)b->tree)-(char *)old)); + top = (DTIndex *) (((char *)new) + (((char *)top)-(char *)old)); } p = &b->tree[b->treeused]; p->keysym = buf[i].keysym; diff --git a/src/xlibi18n/lcDB.c b/src/xlibi18n/lcDB.c index 48a10791..8b02b67e 100644 --- a/src/xlibi18n/lcDB.c +++ b/src/xlibi18n/lcDB.c @@ -517,11 +517,13 @@ append_value_list (void) } if (value != *value_list) { int i; - ssize_t delta; - delta = value - *value_list; + char *old_list; + old_list = *value_list; *value_list = value; + /* Re-derive pointers from the new realloc() result to avoid undefined + behaviour (and crashes on architectures with pointer bounds). */ for (i = 1; i < value_num; ++i) { - value_list[i] += delta; + value_list[i] = value + (value_list[i] - old_list); } }