Skip to content

Commit

Permalink
i#1685 thread init crash: avoid clobbering xsp slot in layered scenario
Browse files Browse the repository at this point in the history
For a thread being in initialized and on the initstack, if a client
init routine invokes library code that triggers a native_exec_syscall
hook, we can end up recursing into another hook which then uses the
mcontext xsp slot for temporary storage.  This clobbers the xsp slot
subsequently used by the client init code.  The solution adopted is to
switch to using the PC slot, which is otherwise unused (only syscall
events present the PC to clients).

Fixes #1685

Review-URL: https://codereview.appspot.com/235030043
  • Loading branch information
derekbruening committed May 4, 2015
1 parent 030d4f3 commit 62f6ed4
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
5 changes: 4 additions & 1 deletion core/lib/instrument.c
Original file line number Diff line number Diff line change
Expand Up @@ -6067,7 +6067,10 @@ dr_get_mcontext_priv(dcontext_t *dcontext, dr_mcontext_t *dmc, priv_mcontext_t *
}
#endif

/* XXX: should we set the pc field? */
/* XXX: should we set the pc field?
* If we do we'll have to adopt a different solution for i#1685 in our Windows
* hooks where today we use the pc slot for temp storage.
*/

return true;
}
Expand Down
10 changes: 6 additions & 4 deletions core/win32/callback.c
Original file line number Diff line number Diff line change
Expand Up @@ -368,10 +368,12 @@ if (!assume_xsp)
endif
# store app xsp in dcontext & switch to dstack; this will be used to save
# app xsp on the switched stack, i.e., dstack; not used after that.
# i#1685: we use the PC slot as it won't affect a new thread that is in the
# middle of init on the initstack and came here during client code.
if TEST(SELFPROT_DCONTEXT, dynamo_options.protect_mask)
mov $MCONTEXT_OFFSET(xcx), xcx
endif
mov xsp, $XSP_OFFSET(xcx)
mov xsp, $PC_OFFSET(xcx)
if TEST(SELFPROT_DCONTEXT, dynamo_options.protect_mask)
mov fs:$TLS_DCONTEXT_OFFSET, xcx
endif
Expand All @@ -383,7 +385,7 @@ if (!assume_xsp)
if TEST(SELFPROT_DCONTEXT, dynamo_options.protect_mask)
mov $MCONTEXT_OFFSET(xcx), xcx
endif
mov $XSP_OFFSET(xcx), xcx
mov $PC_OFFSET(xcx), xcx
push xcx
# need to record stack method, since dcontext could change in handler
Expand Down Expand Up @@ -973,7 +975,7 @@ emit_intercept_code(dcontext_t *dcontext, byte *pc, intercept_function_t callee,
PROT_OFFS));
}
APP(&ilist,
instr_create_save_to_dc_via_reg(dcontext, REG_XCX, REG_XSP, XSP_OFFSET));
instr_create_save_to_dc_via_reg(dcontext, REG_XCX, REG_XSP, PC_OFFSET));
if (TEST(SELFPROT_DCONTEXT, dynamo_options.protect_mask)) {
APP(&ilist, INSTR_CREATE_mov_ld
(dcontext, opnd_create_reg(REG_XCX),
Expand All @@ -992,7 +994,7 @@ emit_intercept_code(dcontext_t *dcontext, byte *pc, intercept_function_t callee,
PROT_OFFS));
}
APP(&ilist,
instr_create_restore_from_dc_via_reg(dcontext, REG_XCX, REG_XCX, XSP_OFFSET));
instr_create_restore_from_dc_via_reg(dcontext, REG_XCX, REG_XCX, PC_OFFSET));
APP(&ilist, INSTR_CREATE_push(dcontext, opnd_create_reg(REG_XCX)));
APP(&ilist,
INSTR_CREATE_push_imm(dcontext, OPND_CREATE_INT32(1)));
Expand Down

0 comments on commit 62f6ed4

Please sign in to comment.