From 8a44e757f0c96e5dcdc71d012f9d0ee168250a05 Mon Sep 17 00:00:00 2001 From: Johan Mattsson <39247600+j-mattsson@users.noreply.github.com> Date: Mon, 16 Dec 2024 18:08:00 +0100 Subject: [PATCH 1/2] Fix too-loose drmgr TLS array limit checks (#7133) In `drmgr.c`, fix potential index out of bounds if users passed in an invalid index as the checks for a too-high limit have a fencepost error. Co-authored-by: Johan Mattsson <39247600+mjunix@users.noreply.github.com> --- ext/drmgr/drmgr.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ext/drmgr/drmgr.c b/ext/drmgr/drmgr.c index 775a02ae5ee..2f678c62424 100644 --- a/ext/drmgr/drmgr.c +++ b/ext/drmgr/drmgr.c @@ -2694,7 +2694,7 @@ static bool drmgr_unreserve_tls_cls_field(bool *taken, int idx) { bool res = false; - if (idx < 0 || idx > MAX_NUM_TLS) + if (idx < 0 || idx >= MAX_NUM_TLS) return false; dr_mutex_lock(tls_lock); if (taken[idx]) { @@ -2726,7 +2726,7 @@ drmgr_get_tls_field(void *drcontext, int idx) { tls_array_t *tls = (tls_array_t *)dr_get_tls_field(drcontext); /* no need to check for tls_taken since would return NULL anyway (i#484) */ - if (idx < 0 || idx > MAX_NUM_TLS || tls == NULL) + if (idx < 0 || idx >= MAX_NUM_TLS || tls == NULL) return NULL; return tls->tls[idx]; } @@ -2736,7 +2736,7 @@ bool drmgr_set_tls_field(void *drcontext, int idx, void *value) { tls_array_t *tls = (tls_array_t *)dr_get_tls_field(drcontext); - if (idx < 0 || idx > MAX_NUM_TLS || tls == NULL) + if (idx < 0 || idx >= MAX_NUM_TLS || tls == NULL) return false; /* going DR's traditional route of efficiency over safety: making this * a debug-only check to avoid cost in release build @@ -2752,7 +2752,7 @@ drmgr_insert_read_tls_field(void *drcontext, int idx, instrlist_t *ilist, instr_ reg_id_t reg) { tls_array_t *tls = (tls_array_t *)dr_get_tls_field(drcontext); - if (idx < 0 || idx > MAX_NUM_TLS || !tls_taken[idx] || tls == NULL) + if (idx < 0 || idx >= MAX_NUM_TLS || !tls_taken[idx] || tls == NULL) return false; if (!reg_is_gpr(reg) || !reg_is_pointer_sized(reg)) return false; @@ -2771,7 +2771,7 @@ drmgr_insert_write_tls_field(void *drcontext, int idx, instrlist_t *ilist, instr reg_id_t reg, reg_id_t scratch) { tls_array_t *tls = (tls_array_t *)dr_get_tls_field(drcontext); - if (idx < 0 || idx > MAX_NUM_TLS || !tls_taken[idx] || tls == NULL) + if (idx < 0 || idx >= MAX_NUM_TLS || !tls_taken[idx] || tls == NULL) return false; if (!reg_is_gpr(reg) || !reg_is_pointer_sized(reg) || !reg_is_gpr(scratch) || !reg_is_pointer_sized(scratch)) @@ -3073,7 +3073,7 @@ void * drmgr_get_cls_field(void *drcontext, int idx) { tls_array_t *tls = (tls_array_t *)dr_get_tls_field(drcontext); - if (idx < 0 || idx > MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) + if (idx < 0 || idx >= MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) return NULL; return tls->cls[idx]; } @@ -3083,7 +3083,7 @@ bool drmgr_set_cls_field(void *drcontext, int idx, void *value) { tls_array_t *tls = (tls_array_t *)dr_get_tls_field(drcontext); - if (idx < 0 || idx > MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) + if (idx < 0 || idx >= MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) return false; tls->cls[idx] = value; return true; @@ -3094,7 +3094,7 @@ void * drmgr_get_parent_cls_field(void *drcontext, int idx) { tls_array_t *tls = (tls_array_t *)dr_get_tls_field(drcontext); - if (idx < 0 || idx > MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) + if (idx < 0 || idx >= MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) return NULL; if (tls->prev != NULL) return tls->prev->cls[idx]; @@ -3107,7 +3107,7 @@ drmgr_insert_read_cls_field(void *drcontext, int idx, instrlist_t *ilist, instr_ reg_id_t reg) { tls_array_t *tls = (tls_array_t *)dr_get_tls_field(drcontext); - if (idx < 0 || idx > MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) + if (idx < 0 || idx >= MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) return false; if (!reg_is_gpr(reg) || !reg_is_pointer_sized(reg)) return false; @@ -3126,7 +3126,7 @@ drmgr_insert_write_cls_field(void *drcontext, int idx, instrlist_t *ilist, instr reg_id_t reg, reg_id_t scratch) { tls_array_t *tls = (tls_array_t *)dr_get_tls_field(drcontext); - if (idx < 0 || idx > MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) + if (idx < 0 || idx >= MAX_NUM_TLS || !cls_taken[idx] || tls == NULL) return false; if (!reg_is_gpr(reg) || !reg_is_pointer_sized(reg) || !reg_is_gpr(scratch) || !reg_is_pointer_sized(scratch)) From 9e2ed1fde68813ffd76e8d0965e877054059bda2 Mon Sep 17 00:00:00 2001 From: Edmund Grimley Evans Date: Mon, 16 Dec 2024 19:15:36 +0000 Subject: [PATCH 2/2] i#3699: Add asm wrapper for 32-bit ARM clone3 in test. (#7137) Add previously missing asm for 32-bit ARM clone3 system call. Issue: #3699 --- suite/tests/linux/clone.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/suite/tests/linux/clone.c b/suite/tests/linux/clone.c index 2d422e336c7..7de35295fa8 100644 --- a/suite/tests/linux/clone.c +++ b/suite/tests/linux/clone.c @@ -340,10 +340,32 @@ make_clone3_syscall(void *clone_args, ulong clone_args_size, void (*fcn)(void)) [clone_args_size] "m"(clone_args_size), [fcn] "m"(fcn) : "x0", "x1", "x2", "x8", "memory"); #elif defined(ARM) - /* XXX: Add asm wrapper for ARM. - * Currently we do not run this test on ARM, so this missing support doesn't - * cause any test failure. + /* The system call number has to go in R7, but R7 is also the frame + * pointer, which means that we are not allowed to include R7 in the + * list of clobbered registers. So we clobber R8 and R9, instead, + * with R7 being saved and restored. + * We use a local variable for sys_clone3 as the intermediate value + * is out of range for ARMv5. */ + long sys_clone3 = CLONE3_SYSCALL_NUM; + asm volatile(".arch armv7-a\n\t" + ".syntax unified\n\t" + "ldr r8, %[sys_clone3]\n\t" + "ldr r0, %[clone_args]\n\t" + "ldr r1, %[clone_args_size]\n\t" + "ldr r2, %[fcn]\n\t" + "mov r9, r7\n\t" + "mov r7, r8\n\t" + "svc #0\n\t" + "mov r7, r9\n\t" + "cbnz r0, 1f\n\t" + "blx r2\n\t" + "1:\n\t" + "str r0, %[result]\n\t" + : [result] "=m"(result) + : [sys_clone3] "m"(sys_clone3), [clone_args] "m"(clone_args), + [clone_args_size] "m"(clone_args_size), [fcn] "m"(fcn) + : "r0", "r1", "r2", "r8", "r9", "memory"); #else # error Unsupported architecture #endif