From b63f1dff9dc736f7fa66f04976436f1f3fe2ac5d Mon Sep 17 00:00:00 2001 From: comex Date: Mon, 23 Feb 2015 00:41:29 -0500 Subject: Ban calls within transform regions in threadsafe mode. --- lib/arm/arch-transform-dis.inc.h | 21 ++++++++------------- lib/arm/dis-arm.inc.h | 7 +++++++ lib/arm/dis-thumb.inc.h | 3 +++ lib/arm/dis-thumb2.inc.h | 18 ++++++++++++++++++ lib/arm64/arch-transform-dis.inc.h | 7 ++----- lib/arm64/assemble.h | 4 ++-- lib/arm64/dis-main.inc.h | 9 +++++++-- lib/hook-functions.c | 3 ++- lib/jump-dis.c | 4 ++++ lib/substitute.h | 8 ++++++++ lib/transform-dis.c | 34 +++++++++++++++++++++++++++++----- lib/transform-dis.h | 5 ++++- lib/x86/arch-transform-dis.inc.h | 9 ++------- lib/x86/dis-main.inc.h | 9 ++++++++- 14 files changed, 104 insertions(+), 37 deletions(-) (limited to 'lib') diff --git a/lib/arm/arch-transform-dis.inc.h b/lib/arm/arch-transform-dis.inc.h index c5963e1..8569300 100644 --- a/lib/arm/arch-transform-dis.inc.h +++ b/lib/arm/arch-transform-dis.inc.h @@ -113,7 +113,7 @@ void transform_dis_data(struct transform_dis_ctx *ctx, unsigned o0, unsigned o1, POPone(actx, scratch); } } - ctx->modify = true; + ctx->base.modify = true; #ifdef TRANSFORM_DIS_VERBOSE printf("transform_dis_data: => %x %x %x %x\n", newval[0], newval[1], newval[2], newval[3]); @@ -149,23 +149,18 @@ void transform_dis_pcrel(struct transform_dis_ctx *ctx, uintptr_t dpc, } static NOINLINE UNUSED -void transform_dis_branch(struct transform_dis_ctx *ctx, - uintptr_t dpc, int cc) { +void transform_dis_branch(struct transform_dis_ctx *ctx, uintptr_t dpc, int cc) { #ifdef TRANSFORM_DIS_VERBOSE printf("transform_dis (0x%llx): branch => 0x%llx\n", (unsigned long long) ctx->base.pc, (unsigned long long) dpc); #endif - if (dpc >= ctx->pc_patch_start && dpc < ctx->pc_patch_end) { - /* don't support this for now */ - /* making the simplifying assumption here that functions will not try - * to branch into the middle of an IT block, which is the case where - * pc_patch_end changes to include additional instructions (as opposed - * to include the end of a partially included instruction, which is - * common) */ - ctx->err = SUBSTITUTE_ERR_FUNC_BAD_INSN_AT_START; - return; - } + /* The check in transform_dis_branch_top is correct under the simplifying + * assumption here that functions will not try to branch into the middle of + * an IT block, which is the case where pc_patch_end changes to include + * additional instructions (as opposed to include the end of a partially + * included instruction, which is common). */ + transform_dis_branch_top(ctx, dpc, cc); struct assemble_ctx actx = tdctx_to_actx(ctx); ctx->write_newop_here = NULL; if ((cc & CC_ARMCC) == CC_ARMCC) { diff --git a/lib/arm/dis-arm.inc.h b/lib/arm/dis-arm.inc.h index 3ae0ab9..ab61877 100644 --- a/lib/arm/dis-arm.inc.h +++ b/lib/arm/dis-arm.inc.h @@ -206,6 +206,13 @@ static INLINE void P(GPR_Rt_addr_offset_none_addr_postidx_reg_Rm_S_1_STRHTr)(tdi static INLINE void P(GPR_dst_B_2_BX)(tdis_ctx ctx, UNUSED struct bitslice dst) { return P(ret)(ctx); } +static INLINE void P(GPR_func_3_BLX)(tdis_ctx ctx, UNUSED struct bitslice func) { + return P(indirect_call)(ctx); +} +static INLINE void P(bl_target_func_2_BL)(tdis_ctx ctx, struct bitslice func) { + return P(branch)(ctx, ctx->base.pc + 8 + sext(bs_get(func, ctx->base.op), 24), + CC_CALL); +} static INLINE void P(dis_arm)(tdis_ctx ctx) { uint32_t op = ctx->base.op = *(uint32_t *) ctx->base.ptr; diff --git a/lib/arm/dis-thumb.inc.h b/lib/arm/dis-thumb.inc.h index 4a2b747..4f758bf 100644 --- a/lib/arm/dis-thumb.inc.h +++ b/lib/arm/dis-thumb.inc.h @@ -81,6 +81,9 @@ static INLINE void P(it_pred_cc_it_mask_mask_1_t2IT)(tdis_ctx ctx, struct bitsli ctx->arch.it_conds[i+2] = (cc_val & ~1) | (mask_val >> (3 - i) & 1); return P(thumb_it)(ctx); } +static INLINE void P(GPR_func_1_tBLXr)(tdis_ctx ctx, UNUSED struct bitslice func) { + return P(indirect_call)(ctx); +} static INLINE void P(thumb_do_it)(tdis_ctx ctx) { uint16_t op = ctx->base.op = *(uint16_t *) ctx->base.ptr; diff --git a/lib/arm/dis-thumb2.inc.h b/lib/arm/dis-thumb2.inc.h index 43ca4ab..5f699c9 100644 --- a/lib/arm/dis-thumb2.inc.h +++ b/lib/arm/dis-thumb2.inc.h @@ -170,6 +170,24 @@ static INLINE void P(unk_Rm_B_2_t2TBB)(tdis_ctx ctx, UNUSED struct bitslice Rm) static INLINE void P(unk_Rt_13_VMOVRRD)(tdis_ctx ctx, UNUSED struct bitslice Rt) { return P(unidentified)(ctx); } +static INLINE void P(t_bltarget_func_1_tBL)(tdis_ctx ctx, struct bitslice func) { + unsigned crap = bs_get(func, ctx->base.op) << 1; + unsigned S = crap >> 24 & 1; + if (S) + crap ^= (3 << 22); + return P(branch)(ctx, ctx->base.pc + 4 + 2 * sext(crap, 25), CC_CALL); + +} +static INLINE void P(t_blxtarget_func_1_tBLXi)(tdis_ctx ctx, struct bitslice func) { + unsigned crap = bs_get(func, ctx->base.op); + unsigned S = crap >> 24 & 1; + if (S) + crap ^= (3 << 22); + return P(branch)(ctx, ctx->base.pc + 4 + 2 * sext(crap, 25), CC_CALL); +} +static INLINE void P(rGPR_func_1_t2BXJ)(tdis_ctx ctx, UNUSED struct bitslice func) { + return P(unidentified)(ctx); +} static INLINE void P(thumb2_do_it)(tdis_ctx ctx) { uint32_t op = ctx->base.op; diff --git a/lib/arm64/arch-transform-dis.inc.h b/lib/arm64/arch-transform-dis.inc.h index ac11e45..123c7ae 100644 --- a/lib/arm64/arch-transform-dis.inc.h +++ b/lib/arm64/arch-transform-dis.inc.h @@ -23,10 +23,7 @@ void transform_dis_branch(struct transform_dis_ctx *ctx, uint_tptr dpc, int cc) (unsigned long long) ctx->base.pc, (unsigned long long) dpc); #endif - if (dpc >= ctx->pc_patch_start && dpc < ctx->pc_patch_end) { - ctx->err = SUBSTITUTE_ERR_FUNC_BAD_INSN_AT_START; - return; - } + transform_dis_branch_top(ctx, dpc, cc); ctx->write_newop_here = NULL; int mov_br_size = size_of_MOVi64(dpc) + 4; @@ -42,7 +39,7 @@ void transform_dis_branch(struct transform_dis_ctx *ctx, uint_tptr dpc, int cc) } int reg = arm64_get_unwritten_temp_reg(&ctx->arch); MOVi64(codep, reg, dpc); - BR(codep, reg); + BR(codep, reg, /*link*/ cc & CC_CALL); } static void transform_dis_pre_dis(UNUSED struct transform_dis_ctx *ctx) {} diff --git a/lib/arm64/assemble.h b/lib/arm64/assemble.h index 1dca7eb..42c0f0c 100644 --- a/lib/arm64/assemble.h +++ b/lib/arm64/assemble.h @@ -56,8 +56,8 @@ static inline void ADRP_ADD(void **codep, int reg, uint64_t pc, uint64_t dpc) { } } -static inline void BR(void **codep, int reg) { - op32(codep, 0xd61f0000 | reg << 5); +static inline void BR(void **codep, int reg, bool link) { + op32(codep, 0xd61f0000 | reg << 5 | link << 21); } static inline void Bccrel(void **codep, int cc, int offset) { diff --git a/lib/arm64/dis-main.inc.h b/lib/arm64/dis-main.inc.h index 0107715..a009189 100644 --- a/lib/arm64/dis-main.inc.h +++ b/lib/arm64/dis-main.inc.h @@ -55,8 +55,13 @@ static INLINE void P(am_ldrlit_label_unk_Rt_6_LDRDl)(tdis_ctx ctx, struct bitsli return P(pcrel)(ctx, ctx->base.pc + sext(bs_get(label, ctx->base.op), 19) * 4, (struct arch_pcrel_info) {bs_get(Rt, ctx->base.op), mode}); } -static INLINE void P(GPR64_Rn_1_RET)(tdis_ctx ctx, UNUSED struct bitslice Rn) { - return P(ret)(ctx); + +static INLINE void P(GPR64_Rn_2_BLR)(tdis_ctx ctx, UNUSED struct bitslice Rn) { + int op = ctx->base.op >> 21 & 3; + if (op == 1) + return P(indirect_call)(ctx); + else + return P(ret)(ctx); } static INLINE void P(dis)(tdis_ctx ctx) { diff --git a/lib/hook-functions.c b/lib/hook-functions.c index faadd82..6d53a0d 100644 --- a/lib/hook-functions.c +++ b/lib/hook-functions.c @@ -207,7 +207,8 @@ int substitute_hook_functions(const struct substitute_function_hook *hooks, * ending make_jump_patch call) */ if ((ret = transform_dis_main(code, &trampoline_ptr, pc_patch_start, &pc_patch_end, (uintptr_t) trampoline_ptr, - &arch, hi->offset_by_pcdiff))) + &arch, hi->offset_by_pcdiff, + thread_safe ? TRANSFORM_DIS_BAN_CALLS : 0))) goto end; uintptr_t dpc = pc_patch_end; diff --git a/lib/jump-dis.c b/lib/jump-dis.c index 528cfc2..e1a7d48 100644 --- a/lib/jump-dis.c +++ b/lib/jump-dis.c @@ -90,6 +90,10 @@ void jump_dis_pcrel(struct jump_dis_ctx *ctx, uint_tptr dpc, ctx->bad_insn = dpc >= ctx->pc_patch_start && dpc < ctx->pc_patch_end; } +static INLINE UNUSED +void jump_dis_indirect_call(UNUSED struct jump_dis_ctx *ctx) { +} + static INLINE UNUSED void jump_dis_ret(struct jump_dis_ctx *ctx) { ctx->continue_after_this_insn = false; diff --git a/lib/substitute.h b/lib/substitute.h index 2045c3d..d8b9fd0 100644 --- a/lib/substitute.h +++ b/lib/substitute.h @@ -29,6 +29,14 @@ enum { * updated to handle that case properly */ SUBSTITUTE_ERR_FUNC_BAD_INSN_AT_START, + /* substitute_hook_functions: can't patch a function because one of the + * instructions within the patch region (other than the last instruction) + * is a call - meaning that a return address within the region (i.e. about + * to point to clobbered code) could be on some thread's stack, where we + * can't easily find and patch it. This check is skipped if + * SUBSTITUTE_NO_THREAD_SAFETY is set. */ + SUBSTITUTE_ERR_FUNC_CALLS_AT_START, + /* substitute_hook_functions: can't patch a function because the (somewhat * cursory) jump analysis found a jump later in the function to within the * patch region at the beginning */ diff --git a/lib/transform-dis.c b/lib/transform-dis.c index 75abc82..4d69da1 100644 --- a/lib/transform-dis.c +++ b/lib/transform-dis.c @@ -12,7 +12,6 @@ struct transform_dis_ctx { /* outputs */ - bool modify; int err; struct dis_ctx_base base; @@ -24,6 +23,8 @@ struct transform_dis_ctx { /* for IT - eww */ bool force_keep_transforming; + bool ban_calls; /* i.e. trying to be thread safe */ + void **rewritten_ptr_ptr; void *write_newop_here; @@ -34,7 +35,14 @@ struct transform_dis_ctx { /* largely similar to jump_dis */ -static INLINE UNUSED +static NOINLINE UNUSED +void transform_dis_indirect_call(struct transform_dis_ctx *ctx) { + /* see error description */ + if (ctx->ban_calls && ctx->base.pc + ctx->base.op_size < ctx->pc_patch_end) + ctx->err = SUBSTITUTE_ERR_FUNC_CALLS_AT_START; +} + +static NOINLINE UNUSED void transform_dis_ret(struct transform_dis_ctx *ctx) { /* ret is okay if it's at the end of the required patch (past the original * patch size is good too) */ @@ -42,7 +50,7 @@ void transform_dis_ret(struct transform_dis_ctx *ctx) { ctx->err = SUBSTITUTE_ERR_FUNC_TOO_SHORT; } -static INLINE UNUSED +static UNUSED void transform_dis_unidentified(UNUSED struct transform_dis_ctx *ctx) { #ifdef TRANSFORM_DIS_VERBOSE printf("transform_dis (0x%llx): unidentified\n", @@ -51,7 +59,7 @@ void transform_dis_unidentified(UNUSED struct transform_dis_ctx *ctx) { /* this isn't exhaustive, so unidentified is fine */ } -static INLINE UNUSED +static NOINLINE UNUSED void transform_dis_bad(struct transform_dis_ctx *ctx) { ctx->err = SUBSTITUTE_ERR_FUNC_BAD_INSN_AT_START; } @@ -61,6 +69,20 @@ void transform_dis_thumb_it(UNUSED struct transform_dis_ctx *ctx) { /* ignore, since it was turned into B */ } +static void transform_dis_branch_top(struct transform_dis_ctx *ctx, + uintptr_t dpc, int cc) { + if (dpc >= ctx->pc_patch_start && dpc < ctx->pc_patch_end) { + /* don't support this for now */ + ctx->err = SUBSTITUTE_ERR_FUNC_BAD_INSN_AT_START; + return; + } + if (cc & CC_CALL) { + transform_dis_indirect_call(ctx); + } else { + transform_dis_ret(ctx); + } +} + static void transform_dis_dis(struct transform_dis_ctx *ctx); static void transform_dis_pre_dis(struct transform_dis_ctx *ctx); static void transform_dis_post_dis(struct transform_dis_ctx *ctx); @@ -71,13 +93,15 @@ int transform_dis_main(const void *restrict code_ptr, uint_tptr *pc_patch_end_p, uint_tptr pc_trampoline, struct arch_dis_ctx *arch_ctx_p, - int *offset_by_pcdiff) { + int *offset_by_pcdiff, + int options) { struct transform_dis_ctx ctx; memset(&ctx, 0, sizeof(ctx)); ctx.pc_patch_start = pc_patch_start; ctx.pc_patch_end = *pc_patch_end_p; ctx.base.pc = pc_patch_start; ctx.arch = *arch_ctx_p; + ctx.ban_calls = options & TRANSFORM_DIS_BAN_CALLS; /* data is written to rewritten both by this function directly and, in case * additional scaffolding is needed, by arch-specific transform_dis_* */ ctx.rewritten_ptr_ptr = rewritten_ptr_ptr; diff --git a/lib/transform-dis.h b/lib/transform-dis.h index 4e853b0..e8969a8 100644 --- a/lib/transform-dis.h +++ b/lib/transform-dis.h @@ -3,10 +3,13 @@ #include #include stringify(TARGET_DIR/arch-dis.h) +#define TRANSFORM_DIS_BAN_CALLS 1 + int transform_dis_main(const void *restrict code_ptr, void **restrict rewritten_ptr_ptr, uint_tptr pc_patch_start, uint_tptr *pc_patch_end_p, uint_tptr pc_trampoline, struct arch_dis_ctx *arch_ctx_p, - int *offset_by_pcdiff); + int *offset_by_pcdiff, + int options); diff --git a/lib/x86/arch-transform-dis.inc.h b/lib/x86/arch-transform-dis.inc.h index 56e41eb..a4be424 100644 --- a/lib/x86/arch-transform-dis.inc.h +++ b/lib/x86/arch-transform-dis.inc.h @@ -81,10 +81,7 @@ static void transform_dis_branch(struct transform_dis_ctx *ctx, uint_tptr dpc, *ctx->rewritten_ptr_ptr = code; return; } - if (dpc >= ctx->pc_patch_start && dpc < ctx->pc_patch_end) { - ctx->err = SUBSTITUTE_ERR_FUNC_BAD_INSN_AT_START; - return; - } + transform_dis_branch_top(ctx, dpc, cc); void *code = *ctx->rewritten_ptr_ptr; struct arch_dis_ctx arch; @@ -109,10 +106,8 @@ static void transform_dis_branch(struct transform_dis_ctx *ctx, uint_tptr dpc, transform_dis_ret(ctx); } else { ctx->write_newop_here = NULL; - make_jmp_or_call(&code, ctx->pc_trampoline, dpc, cc & CC_CALL); - if (!(cc & CC_CALL)) - transform_dis_ret(ctx); + make_jmp_or_call(&code, ctx->pc_trampoline, dpc, cc & CC_CALL); } *ctx->rewritten_ptr_ptr = code; } diff --git a/lib/x86/dis-main.inc.h b/lib/x86/dis-main.inc.h index 9959409..b6064e9 100644 --- a/lib/x86/dis-main.inc.h +++ b/lib/x86/dis-main.inc.h @@ -44,6 +44,7 @@ VEX last byte 1:0: {none, 66, f3, f2} #define I_JIMM_ONLY 0x80 /* imm is jump offset */ #define I_JIMM (0x80|I_JMP) #define I_BAD 0x80 +#define I_CALL 0x100 /* not really in the table */ #ifdef TARGET_x86_64 #define if64(_64, _32) _64 #else @@ -119,7 +120,7 @@ static void P(dis)(tdis_ctx ctx) { int mod, rm = 0; restart:; uint8_t byte1 = *ptr++; - uint8_t bits = onebyte_bits[byte1]; + int bits = onebyte_bits[byte1]; /* printf("b1=%x bytes=%x\n", byte1, bits); */ if ((bits & I_TYPE_MASK) == I_SPEC) { if (byte1 == 0x0f) { @@ -134,6 +135,8 @@ restart:; int subop = modrm >> 3 & 7; if (subop == 4 || subop == 5) /* JMP */ bits = I_JMP | I_MODA; + else if (subop == 2 || subop == 3) /* CALL */ + bits = I_CALL | I_MODA; else bits = I_MODA; } else { @@ -283,6 +286,8 @@ got_bits: UNUSED #ifdef TARGET_x86_64 } else if ((bits & I_MODA) == I_MODA && mod == 0 && rm == 5) { int32_t disp = *(int32_t *) (orig + modrm_off + 1); + if (bits & I_CALL) + P(indirect_call)(ctx); /* unlike ARM, we can always switch to non-pcrel without making the * instruction from scratch, so we don't have 'reg' and 'lm' */ struct arch_pcrel_info info = { @@ -319,6 +324,8 @@ got_bits: UNUSED #endif } else if ((bits & I_TYPE_MASK) == I_JMP) { P(ret)(ctx); + } else if (bits & I_CALL) { + P(indirect_call)(ctx); } else { P(unidentified)(ctx); } -- cgit v1.2.3