Add protection against sigreturn oriented programming (SROP).

This change hardens against invalid calls to sigreturn, which is a very
useful gadget when compromising a process. The system call now verifies
it is a real return from a signal and aborts the process otherwise. This
should render such attacks impossible in threads that are not servicing a
signal, and infeasible in threads that are handling signals they are yet to
return from.

The kernel now keeps track for each thread how many signals are being
handled but haven't returned yet.

Each thread now has a random signal value. It is re-randomized when the
thread handles a signal and the current signal counter is zero. This is
xorred with the context address and used as canary on the stack during
signal dispatch, protecting the saved context on the stack. This works
mostly like the regular stack protector.

The kernel now keeps track of the stack pointer for a single handled
signal per thread. It doesn't seem worth it to keep track of multiple
handled signals, as more than one is rare. Note that each delivered signal
will not necessarily result in a sigreturn because it is valid for a thread
to longjmp(3) out of a signal handler to a valid jmp_buf.

The sigreturn system call will abort if either:

- It was not called from the kernel sigreturn page.
- The thread is not currently processing a signal.
- The thread is processing a single signal, and the stack pointer did not
  have the expected value.
- It fails to read the context on the stack.
- The canary is wrong.

kernel/include/sortix/kernel/thread.h

@@ -76,9 +76,13 @@ public:
 	stack_t signal_stack;
 	addr_t kernelstackpos;
 	size_t kernelstacksize;
+	size_t signal_count;
+	uintptr_t signal_single_frame;
+	uintptr_t signal_canary;
 	bool kernelstackmalloced;
 	bool pledged_destruction;
 	bool force_no_signals;
+	bool signal_single;
 	Clock execute_clock;
 	Clock system_clock;
 

kernel/process.cpp

@@ -1689,7 +1689,7 @@ void sys_scram(int event, const void* user_info)
 			(intmax_t) process->pid,
 			event);
 	}
-	// TODO: Allow debugging this event.
+	// TODO: Allow debugging this event (and see signal.cpp sigreturn).
 	kthread_exit();
 }
 

kernel/signal.cpp

@@ -23,6 +23,7 @@
 #include <errno.h>
 #include <string.h>
 #include <stdint.h>
+#include <stdlib.h>
 #include <signal.h>
 
 #include <sortix/sigaction.h>
@@ -508,6 +509,7 @@ struct stack_frame
 	siginfo_t* siginfo_param;
 	ucontext_t* ucontext_param;
 	void* cookie_param;
+	uintptr_t canary;
 	ucontext_t ucontext;
 	siginfo_t siginfo;
 };
@@ -516,6 +518,7 @@ struct stack_frame
 {
 	uintptr_t misalignment[1];
 	uintptr_t sigreturn;
+	uintptr_t canary;
 	ucontext_t ucontext;
 	siginfo_t siginfo;
 };
@@ -728,6 +731,12 @@ retry_another_signal:
 #error "You need to format the stack frame"
 #endif
 
+	// Store a canary so it can later be verified this is a real signal being
+	// returned from.
+	if ( signal_count == 0 )
+		arc4random_buf(&signal_canary, sizeof(signal_canary));
+	stack_frame.canary = signal_canary ^ (uintptr_t) stack;
+
 	// Format the siginfo into the stack frame.
 	stack_frame.siginfo.si_signo = signum;
 #if defined(__i386__) || defined(__x86_64__)
@@ -783,6 +792,17 @@ retry_another_signal:
 		}
 	}
 
+	// Know for sure when there's no signal still being handled. This is an
+	// over-approximation as programs may do absolutely awful things such as
+	// longjmp(3)ing out of signal handlers, so each delivered signal does not
+	// nessesarily mean a sigreturn. This will work correctly in reasonable
+	// programs though and will harden those programs. Remember the stack frame
+	// as well for verification if there is no recursive signal handling.
+	if ( signal_count != SIZE_MAX )
+		signal_count++;
+	if ( (signal_single = signal_count == 1) )
+		signal_single_frame = (uintptr_t) stack;
+
 	// Run the signal handler by returning to user-space.
 	return;
 }
@@ -792,31 +812,100 @@ void Thread::HandleSigreturn(struct interrupt_context* intctx)
 	assert(Interrupt::IsEnabled());
 	assert(this == CurrentThread());
 
-	ScopedLock lock(&process->signal_lock);
-
 	struct stack_frame stack_frame;
-	const struct stack_frame* user_stack_frame;
+	struct stack_frame* user_stack_frame;
 #if defined(__i386__)
-	user_stack_frame = (const struct stack_frame*)
+	user_stack_frame = (struct stack_frame*)
 		(intctx->esp - offsetof(struct stack_frame, sigreturn) - 4);
+	bool wrong_ip = intctx->eip != (uintptr_t) process->sigreturn + 2;
 #elif defined(__x86_64__)
-	user_stack_frame = (const struct stack_frame*)
+	user_stack_frame = (struct stack_frame*)
 		(intctx->rsp - offsetof(struct stack_frame, sigreturn) - 8);
+	bool wrong_ip = intctx->rip != (uintptr_t) process->sigreturn + 2;
 #else
 #error "You need to locate the stack we passed the signal handler"
 #endif
 
-	if ( CopyFromUser(&stack_frame, user_stack_frame, sizeof(stack_frame)) )
+	// Protect against sigreturn oriented programming (SROP) as described in
+	// "Framing Signals - A Return to Portable Shellcode" (Bosman and Bos 2014).
+
+	// If the we're not called from the kernel sigreturn page, it is not a valid
+	// sigreturn.
+	if ( wrong_ip )
 	{
-		memcpy(&signal_mask, &stack_frame.ucontext.uc_sigmask, sizeof(signal_mask));
-		memcpy(&signal_stack, &stack_frame.ucontext.uc_stack, sizeof(signal_stack));
-		signal_stack.ss_flags &= __SS_SUPPORTED_FLAGS;
-		struct thread_registers resume_regs;
-		Scheduler::SaveInterruptedContext(intctx, &resume_regs);
-		DecodeMachineContext(&stack_frame.ucontext.uc_mcontext, &resume_regs);
-		Scheduler::LoadInterruptedContext(intctx, &resume_regs);
+		process->ExitThroughSignal(SIGABRT);
+		Log::PrintF("%s[%ji]: sigreturn smashing detected: Bypassed kernel sigreturn page\n",
+			process->program_image_path,
+			(intmax_t) process->pid);
+		// TODO: Allow debugging this event (see scram(2)).
+		kthread_exit();
 	}
 
+	// If no signals are being serviced by this thread at the moment, it is not
+	// a valid sigreturn.
+	if ( signal_count == 0 )
+	{
+		process->ExitThroughSignal(SIGABRT);
+		Log::PrintF("%s[%ji]: sigreturn smashing detected: Thread wasn't servicing a signal\n",
+			process->program_image_path,
+			(intmax_t) process->pid);
+		// TODO: Allow debugging this event (see scram(2)).
+		kthread_exit();
+	}
+
+	// If a single signal is being serviced by this thread at the moment, the
+	// stack pointer must be what we expect it to be. If there's multiple, we
+	// don't know which one is the correct. (We could keep track of them and
+	// ensure it's one of them, but that's not really worth it. The list of such
+	// delivered signals could grow without bound because it's valid to longjmp
+	// out of a signal handler)
+	if ( signal_single && (uintptr_t) user_stack_frame != signal_single_frame )
+	{
+		process->ExitThroughSignal(SIGABRT);
+		Log::PrintF("%s[%ji]: sigreturn smashing detected: Stack pointer was wrong\n",
+			process->program_image_path,
+			(intmax_t) process->pid);
+		// TODO: Allow debugging this event (see scram(2)).
+		kthread_exit();
+	}
+
+	// If we couldn't read the frame, the sigreturn is certainly bad.
+	if ( !CopyFromUser(&stack_frame, user_stack_frame, sizeof(stack_frame)) )
+	{
+		process->ExitThroughSignal(SIGABRT);
+		Log::PrintF("%s[%ji]: sigreturn smashing detected: Couldn't read stack frame: %m\n",
+			process->program_image_path,
+			(intmax_t) process->pid);
+		// TODO: Allow debugging this event (see scram(2)).
+		kthread_exit();
+	}
+	ZeroUser(user_stack_frame, sizeof(*user_stack_frame));
+
+	// If the random canary isn't correct, the sigreturn is certianly bad.
+	if ( stack_frame.canary != (signal_canary ^ (uintptr_t) user_stack_frame) )
+	{
+		process->ExitThroughSignal(SIGABRT);
+		Log::PrintF("%s[%ji]: sigreturn smashing detected: Verification value was incorrect\n",
+			process->program_image_path,
+			(intmax_t) process->pid);
+		// TODO: Allow debugging this event (see scram(2)).
+		kthread_exit();
+	}
+
+	ScopedLock lock(&process->signal_lock);
+
+	memcpy(&signal_mask, &stack_frame.ucontext.uc_sigmask, sizeof(signal_mask));
+	memcpy(&signal_stack, &stack_frame.ucontext.uc_stack, sizeof(signal_stack));
+	signal_stack.ss_flags &= __SS_SUPPORTED_FLAGS;
+	struct thread_registers resume_regs;
+	Scheduler::SaveInterruptedContext(intctx, &resume_regs);
+	DecodeMachineContext(&stack_frame.ucontext.uc_mcontext, &resume_regs);
+	Scheduler::LoadInterruptedContext(intctx, &resume_regs);
+
+	if ( signal_count != SIZE_MAX )
+		signal_count--;
+	signal_single = false;
+
 	UpdatePendingSignals(this);
 	intctx->signal_pending = asm_signal_is_pending;
 

kernel/thread.cpp

@@ -89,9 +89,13 @@ Thread::Thread()
 	memset(&registers, 0, sizeof(registers));
 	kernelstackpos = 0;
 	kernelstacksize = 0;
+	signal_count = 0;
+	signal_single_frame = 0;
+	signal_canary = 0;
 	kernelstackmalloced = false;
 	pledged_destruction = false;
 	force_no_signals = false;
+	signal_single = false;
 	sigemptyset(&signal_pending);
 	sigemptyset(&signal_mask);
 	memset(&signal_stack, 0, sizeof(signal_stack));