Message ID | BLU436-SMTP91D615856A23994E55223E979A0@phx.gbl |
---|---|
State | New |
Headers | show |
Oh, yeah. abort is the generic fallback for __builtin_trap. Certainly just supporting __builtin_trap directly in a compiler backend (with no outcall) is best. But this issue might come up in other places or affect other machines, and the set of machines and compilers we support for building libc today most likely includes others that don't have a proper __builtin_trap. So we might as well fix abort in rtld. We already have __assert_fail in elf/dl-minimal.c for similar issues with enabling assert in rtld. We could just add an abort there. It would be ideal to avoid any actual abort calls in our own source code sneaking into rtld, because it's far better if they're _dl_fatal_printf so there's a message. But I can't think of a reasonable way to make compiler-generated abort calls link while preventing explicit abort calls in the source from linking, so we can live without that ideal.
On Fri, Oct 31, 2014 at 07:35:59PM -0700, Roland McGrath wrote: > Oh, yeah. abort is the generic fallback for __builtin_trap. Certainly > just supporting __builtin_trap directly in a compiler backend (with no > outcall) is best. But this issue might come up in other places or affect > other machines, and the set of machines and compilers we support for > building libc today most likely includes others that don't have a proper > __builtin_trap. While glibc has to work around it for the time being (either by not using __builtin_trap, or by providing abort here), I think it would be nice to press for fixing this on the compiler side. There's no good reason for the compiler to generate a call to abort when every ISA I've ever seen has HCF-type instructions that can be used directly with no dependency on a function call. Aside from the ldso issues, __builtin_trap or similar compiler-generated traps might be used in places where the call is impossible, like handling a fatal condition where the GOT is corrupt, where the TCB is corrupt (in which case abort can't properly raise signals for the calling thread), or where the function pointer used to make syscalls (via vdso on x86, for instance) is corrupt. In the worst case, this corruption was caused by an attacker who has happily put the address of shellcode at one of the above places. Rich
> While glibc has to work around it for the time being (either by not > using __builtin_trap, or by providing abort here), I think it would be > nice to press for fixing this on the compiler side. There's no good > reason for the compiler to generate a call to abort when every ISA > I've ever seen has HCF-type instructions that can be used directly > with no dependency on a function call. Aside from the ldso issues, > __builtin_trap or similar compiler-generated traps might be used in > places where the call is impossible, like handling a fatal condition > where the GOT is corrupt, where the TCB is corrupt (in which case > abort can't properly raise signals for the calling thread), or where > the function pointer used to make syscalls (via vdso on x86, for > instance) is corrupt. In the worst case, this corruption was caused by > an attacker who has happily put the address of shellcode at one of the > above places. It's very straightforward to define it for any machine that lacks it. I haven't actually done a survey of existing GCC backends. You might want to do that and report bugs to individual backends.
Index: config/pa/pa.md =================================================================== --- config/pa/pa.md (revision 216987) +++ config/pa/pa.md (working copy) @@ -123,7 +123,7 @@ ;; type "binary" insns have two input operands (1,2) and one output (0) (define_attr "type" - "move,unary,binary,shift,nullshift,compare,load,store,uncond_branch,branch,cbranch,fbranch,call,sibcall,dyncall,fpload,fpstore,fpalu,fpcc,fpmulsgl,fpmuldbl,fpdivsgl,fpdivdbl,fpsqrtsgl,fpsqrtdbl,multi,milli,sh_func_adrs,parallel_branch,fpstore_load,store_fpload" + "move,unary,binary,shift,nullshift,compare,load,store,uncond_branch,branch,cbranch,fbranch,call,sibcall,dyncall,fpload,fpstore,fpalu,fpcc,fpmulsgl,fpmuldbl,fpdivsgl,fpdivdbl,fpsqrtsgl,fpsqrtdbl,multi,milli,sh_func_adrs,parallel_branch,fpstore_load,store_fpload,trap" (const_string "binary")) (define_attr "pa_combine_type" @@ -175,7 +175,7 @@ ;; Disallow instructions which use the FPU since they will tie up the FPU ;; even if the instruction is nullified. (define_attr "in_nullified_branch_delay" "false,true" - (if_then_else (and (eq_attr "type" "!uncond_branch,branch,cbranch,fbranch,call,sibcall,dyncall,multi,milli,sh_func_adrs,fpcc,fpalu,fpmulsgl,fpmuldbl,fpdivsgl,fpdivdbl,fpsqrtsgl,fpsqrtdbl,parallel_branch") + (if_then_else (and (eq_attr "type" "!uncond_branch,branch,cbranch,fbranch,call,sibcall,dyncall,multi,milli,sh_func_adrs,fpcc,fpalu,fpmulsgl,fpmuldbl,fpdivsgl,fpdivdbl,fpsqrtsgl,fpsqrtdbl,parallel_branch,trap") (eq_attr "length" "4") (not (match_test "RTX_FRAME_RELATED_P (insn)"))) (const_string "true") @@ -183,7 +183,7 @@ ;; For calls and millicode calls. (define_attr "in_call_delay" "false,true" - (if_then_else (and (eq_attr "type" "!uncond_branch,branch,cbranch,fbranch,call,sibcall,dyncall,multi,milli,sh_func_adrs,parallel_branch") + (if_then_else (and (eq_attr "type" "!uncond_branch,branch,cbranch,fbranch,call,sibcall,dyncall,multi,milli,sh_func_adrs,parallel_branch,trap") (eq_attr "length" "4") (not (match_test "RTX_FRAME_RELATED_P (insn)"))) (const_string "true") @@ -5324,6 +5324,15 @@ [(set_attr "type" "binary,binary") (set_attr "length" "4,4")]) +;; Trap instructions. + +(define_insn "trap" + [(trap_if (const_int 1) (const_int 0))] + "" + "{addit|addi,tc},<> 1,%%r0,%%r0" + [(set_attr "type" "trap") + (set_attr "length" "4")]) + ;; Clobbering a "register_operand" instead of a match_scratch ;; in operand3 of millicode calls avoids spilling %r1 and ;; produces better code.