[PATCH] replace 'hlt' by 'ud2' in i386/x32/x86_64 a_crash implementations

[PATCH] replace 'hlt' by 'ud2' in i386/x32/x86_64 a_crash implementations

[Alexander Monakov]

---
ud2 is also what GCC and Clang use to implement __builtin_trap()

ud2 causes SIGILL rather than SIGSEGV; SIGSEGV is more likely to have a custom
signal handler installed

 arch/i386/atomic.h   | 2 +-
 arch/x32/atomic.h    | 2 +-
 arch/x86_64/atomic.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/i386/atomic.h b/arch/i386/atomic.h
index 4fe7bde..2f78d31 100644
--- a/arch/i386/atomic.h
+++ b/arch/i386/atomic.h
@@ -103,7 +103,7 @@ static inline void a_barrier()
 
 static inline void a_crash()
 {
-	__asm__ __volatile__( "hlt" : : : "memory" );
+	__asm__ __volatile__( "ud2" : : : "memory" );
 }
 
 
diff --git a/arch/x32/atomic.h b/arch/x32/atomic.h
index 333098c..a8e7603 100644
--- a/arch/x32/atomic.h
+++ b/arch/x32/atomic.h
@@ -98,7 +98,7 @@ static inline void a_barrier()
 
 static inline void a_crash()
 {
-	__asm__ __volatile__( "hlt" : : : "memory" );
+	__asm__ __volatile__( "ud2" : : : "memory" );
 }
 
 
diff --git a/arch/x86_64/atomic.h b/arch/x86_64/atomic.h
index 333098c..a8e7603 100644
--- a/arch/x86_64/atomic.h
+++ b/arch/x86_64/atomic.h
@@ -98,7 +98,7 @@ static inline void a_barrier()
 
 static inline void a_crash()
 {
-	__asm__ __volatile__( "hlt" : : : "memory" );
+	__asm__ __volatile__( "ud2" : : : "memory" );
 }
 
 
-- 
2.1.3

Re: [PATCH] replace 'hlt' by 'ud2' in i386/x32/x86_64 a_crash implementations

[Rich Felker]

On Thu, Apr 16, 2015 at 08:58:59PM +0300, Alexander Monakov wrote:
> ---
> ud2 is also what GCC and Clang use to implement __builtin_trap()
> 
> ud2 causes SIGILL rather than SIGSEGV; SIGSEGV is more likely to have a custom
> signal handler installed

I've pondered instead using:

	push $-1
	mov $175,%eax
	xor %ebx,%ebx
	mov %esp,%ecx
	int $128
	hlt // or ud2?

This should be uncatchable but it's moderately larger. 

Rich

Re: [PATCH] replace 'hlt' by 'ud2' in i386/x32/x86_64 a_crash implementations

[Alexander Monakov]

On Fri, 17 Apr 2015, Rich Felker wrote:

> On Thu, Apr 16, 2015 at 08:58:59PM +0300, Alexander Monakov wrote:
> > ---
> > ud2 is also what GCC and Clang use to implement __builtin_trap()
> > 
> > ud2 causes SIGILL rather than SIGSEGV; SIGSEGV is more likely to have a custom
> > signal handler installed
> 
> I've pondered instead using:
> 
> 	push $-1
> 	mov $175,%eax
> 	xor %ebx,%ebx
> 	mov %esp,%ecx
> 	int $128
> 	hlt // or ud2?
> 
> This should be uncatchable but it's moderately larger. 

An argument in favor of plain hlt/ud2 is that registers and memory are
preserved, in case someone will be analyzing the coredump.

Alexander

Re: [PATCH] replace 'hlt' by 'ud2' in i386/x32/x86_64 a_crash implementations

[Rich Felker]

On Fri, Apr 17, 2015 at 11:12:20AM +0300, Alexander Monakov wrote:
> On Fri, 17 Apr 2015, Rich Felker wrote:
> 
> > On Thu, Apr 16, 2015 at 08:58:59PM +0300, Alexander Monakov wrote:
> > > ---
> > > ud2 is also what GCC and Clang use to implement __builtin_trap()
> > > 
> > > ud2 causes SIGILL rather than SIGSEGV; SIGSEGV is more likely to have a custom
> > > signal handler installed
> > 
> > I've pondered instead using:
> > 
> > 	push $-1
> > 	mov $175,%eax
> > 	xor %ebx,%ebx
> > 	mov %esp,%ecx
> > 	int $128
> > 	hlt // or ud2?
> > 
> > This should be uncatchable but it's moderately larger. 
> 
> An argument in favor of plain hlt/ud2 is that registers and memory are
> preserved, in case someone will be analyzing the coredump.

That's a good point. The above could be fixed to preserve registers
with some minor added push/pop (simple pusha/popa on 32-bit)
before/after the syscall, but then it gets bigger.

Short of doing something like that, I think it's something of a flip
which of SIGSEGV vs SIGILL is better, and I don't feel like we have
enough information to make a good decision. There are reasons either
could be trapped -- some programs probably trap SIGILL to do runtime
cpudetection nonsense, and some programs just trap all fatal signals
to try to 'cleanup' or save state at exit. I'm not opposed to changing
to ud2 if it's better but I'd like to feel confident that it is better
rather than just guessing. Right now the main argument for ud2 is just
consistency with gcc (which isn't a bad reason in itself, but not very
strong either).

Rich

Re: [PATCH] replace 'hlt' by 'ud2' in i386/x32/x86_64 a_crash implementations

[Alexander Monakov]

> > An argument in favor of plain hlt/ud2 is that registers and memory are
> > preserved, in case someone will be analyzing the coredump.
> 
> That's a good point. The above could be fixed to preserve registers
> with some minor added push/pop (simple pusha/popa on 32-bit)
> before/after the syscall, but then it gets bigger.

Hm, no, I wouldn't like to see that.  By the time a_crash executes, "the
impossible happened", so stack pointer might be pointing somewhere you
wouldn't want modified.

A good reason for compilers to generate ud2 is that it also works for kernel
code, unlike hlt -- but then OSv carries a copy of musl to run it in the
context of, as I understand, virtualized kernel -- what happens when they
execute a_crash?

Alexander.

Re: [PATCH] replace 'hlt' by 'ud2' in i386/x32/x86_64 a_crash implementations

[Rich Felker]

On Fri, Apr 17, 2015 at 09:48:38PM +0300, Alexander Monakov wrote:
> > > An argument in favor of plain hlt/ud2 is that registers and memory are
> > > preserved, in case someone will be analyzing the coredump.
> > 
> > That's a good point. The above could be fixed to preserve registers
> > with some minor added push/pop (simple pusha/popa on 32-bit)
> > before/after the syscall, but then it gets bigger.
> 
> Hm, no, I wouldn't like to see that.  By the time a_crash executes, "the
> impossible happened", so stack pointer might be pointing somewhere you
> wouldn't want modified.

Given that it's used in C code and C code generally can't be running
at all without a stack, I don't think that's an issue. We're not using
it for detection of stack overflows (which are handled with guard
pages) but for detection of corrupt heap structures, etc. It's
possible that a heap overflow could cause the stack pointer to be
restored to the wrong value, but then I think you would have a really
hard time reaching the a_crash() call.

> A good reason for compilers to generate ud2 is that it also works for kernel
> code, unlike hlt -- but then OSv carries a copy of musl to run it in the
> context of, as I understand, virtualized kernel -- what happens when they
> execute a_crash?

Yes, that does seem like a good aspect.

Rich