diff options
author | Kaz Kylheku <kaz@kylheku.com> | 2019-12-17 16:44:52 -0800 |
---|---|---|
committer | Kaz Kylheku <kaz@kylheku.com> | 2019-12-17 16:44:52 -0800 |
commit | 1325adcc2219a2052bb646deea2080f1d76364d9 (patch) | |
tree | 443eea3fedbd65ec45568380ecae2f5d909aebf5 /signal.h | |
parent | b8f8be2738e304f1b857b0c1be7abe77f1c622e7 (diff) | |
download | txr-1325adcc2219a2052bb646deea2080f1d76364d9.tar.gz txr-1325adcc2219a2052bb646deea2080f1d76364d9.tar.bz2 txr-1325adcc2219a2052bb646deea2080f1d76364d9.zip |
bugfix: crash in extended_setjmp due to PIE.
A crash occurs on Ubuntu 18.04, 32 bit x86, when executing the
test case tests/007/except-2.txr, whereby TXR segfaults in the
v_try function.
This is reminiscent of a January, 2016 report in the txr-users
mailing list from Morit Barsnick, who also ran into a crash in
the same test case.
Background: it appears that the compiler in Ubuntu 18.04
enables PIE (position-independent executables) by default. Thus
even simple executables that are not shared libraries
reference their own global variables through an offset table,
instead of direct addressing. To access globals, the compiler
has to emit code that retrieves their addresses from a table,
pulling them into a register, and then performing indirect
memory accesses through the register. Sometimes the emitted
code doesn't keep these addresses in a register. The address
of a global variable accessed multiple times in a block of
code may get spilled from a register into the stack, and then
later retrieved from the stack again to access that same
global.
In our extended_setjmp logic, we save the values of a few
global variables and restore them if the extended_longjmp
takes place to return to that point. The problem is that when
restoring some of the globals, the compiler is relying on
retrieving the effective addresses from the temporary spill
locations in the stack. However, those temporary locations
have since been re-used for other purposes and the access to
the globals therefore crashes or produces unpredictable
results.
Essentially, it's as if GCC did this around our code:
{
unsigned *debug_enable_addr = &debug_enable;
/* save and restore logic here uses *debug_enable_addr
* to refer to debug_enable
*/
if (extended_setjmp(...))
...
}
/* Oops, debug_enable_addr is now garbage!
* We are jumping back into the scope which will try to use
* its value to restore the debug_enable global.
*/
extended_longjmp(...);
I have experimented with a few approaches that did not work, and settled
on moving the code which saves and restores the globals into functions.
GCC will not cache the effective address calculation of a global
variable access between calls to different external functions which
access that variable.
The mitigation in this commit gets the test cases to pass even
if TXR is compiled with PIE. However, PIE should be disabled.
Not only does it cause the above problem, but it has a huge
performance impact: a more than 16% slowdown, which is quite
unacceptable.
* eval.h (dyn_env): Delare here. Some sources were depending on signal.h
providing this, which is wrong. Now signal.h doesn't declare it any
longer.
* signal.h (EJ_DBG_SAVE, EJ_DBG_REST): Macros removed.
(extended_setjmp): Greatly simplified. Extended restoring logic is now
done in extended_longjmp, and the extended save for the globals
is a function call. Just moving the restore into extended_longjmp
probably would have fixed this issue.
(extended_longjmp): Call extjmp_restore.
(extjmp_save, extjmp_restore): Declared.
* unwind.c (extjmp_save, extjmp_restore): New functions.
Diffstat (limited to 'signal.h')
-rw-r--r-- | signal.h | 74 |
1 files changed, 19 insertions, 55 deletions
@@ -26,21 +26,6 @@ */ -#if CONFIG_DEBUG_SUPPORT -extern unsigned debug_state; -#define EJ_DBG_MEMB int ds; -#define EJ_DBG_SAVE(EJB) ((EJB).ds = debug_state), -#define EJ_DBG_REST(EJB) (debug_state = (EJB).ds), -#else -#define EJ_DBG_MEMB -#define EJ_DBG_SAVE(EJB) -#define EJ_DBG_REST(EJB) -#endif - -#define EJ_OPT_MEMB EJ_DBG_MEMB -#define EJ_OPT_SAVE(EJB) EJ_DBG_SAVE(EJB) -#define EJ_OPT_REST(EJB) EJ_DBG_REST(EJB) - #if __i386__ struct jmp { @@ -172,12 +157,22 @@ void jmp_restore(struct jmp *, int); } #endif +#if CONFIG_DEBUG_SUPPORT +#define EJ_DBG_MEMB int ds; +#else +#define EJ_DBG_MEMB +#endif + +#define EJ_OPT_MEMB EJ_DBG_MEMB + #if HAVE_POSIX_SIGS typedef struct { unsigned int set; } small_sigset_t; +extern small_sigset_t sig_blocked_cache; + #define sig_save_enable \ do { \ int sig_save = async_sig_enabled; \ @@ -225,29 +220,6 @@ typedef struct { volatile int rv; } extended_jmp_buf; -#define extended_setjmp(EJB) \ - (jmp_save(&(EJB).jb) \ - ? (async_sig_enabled = (EJB).se, \ - dyn_env = (EJB).de, \ - gc_enabled = (EJB).gc, \ - gc_prot_top = (EJB).gc_pt, \ - sig_mask(SIG_SETMASK, \ - strip_qual(small_sigset_t *, \ - &(EJB).blocked), 0), \ - EJ_OPT_REST(EJB) \ - (EJB).rv) \ - : ((EJB).se = async_sig_enabled, \ - (EJB).de = dyn_env, \ - (EJB).gc = gc_enabled, \ - (EJB).gc_pt = gc_prot_top, \ - (EJB).blocked.set = sig_blocked_cache.set,\ - EJ_OPT_SAVE(EJB) \ - 0)) - -#define extended_longjmp(EJB, ARG) \ - ((EJB).rv = (ARG), jmp_restore(&(EJB).jb, 1)) - -extern small_sigset_t sig_blocked_cache; extern volatile sig_atomic_t async_sig_enabled; #else @@ -269,28 +241,20 @@ typedef struct { volatile int rv; } extended_jmp_buf; -#define extended_setjmp(EJB) \ - (jmp_save(&(EJB).jb) \ - ? (dyn_env = (EJB).de, \ - gc_enabled = ((EJB).gc), \ - gc_prot_top = (EJB).gc_pt, \ - EJ_OPT_REST(EJB) \ - (EJB).rv) \ - : ((EJB).de = dyn_env, \ - (EJB).gc = gc_enabled, \ - (EJB).gc_pt = gc_prot_top, \ - EJ_OPT_SAVE(EJB) \ - 0)) - -#define extended_longjmp(EJB, ARG) \ - ((EJB).rv = (ARG), jmp_restore(&(EJB).jb, 1)) - extern int async_sig_enabled; #endif -extern val dyn_env; /* eval.c */ +#define extended_setjmp(EJB) \ + (jmp_save(&(EJB).jb) \ + ? ((EJB).rv) \ + : (extjmp_save(&(EJB)), 0)) + +#define extended_longjmp(EJB, ARG) \ + ((EJB).rv = (ARG), extjmp_restore(&(EJB)), jmp_restore(&(EJB).jb, 1)) +void extjmp_save(extended_jmp_buf *ejb); +void extjmp_restore(extended_jmp_buf *); void sig_init(void); val set_sig_handler(val signo, val lambda); val get_sig_handler(val signo); |