UML - kill processes instead of panicing kernel

PATCH In review Jeff Dike 2 participants 2 messages Nov 20, 2007 User-mode Linux (UML)

Share & tools

Jeff Dike

author

2007-11-20 18:26 UTC #0 lore raw

PATCH 3/4 UML - kill processes instead of panicing kernel

UML was panicing in the case of failures of libc calls which shouldn't happen. This is an overreaction since a failure from libc doesn't normally mean that kernel data structures are in an unknown state. Instead, the current process should just be killed if there is no way to recover.

The case that prompted this was a failure of PTRACE_SETREGS restoring the same state that was read by PTRACE_GETREGS. It appears that when a process tries to load a bogus value into a segment register, it segfaults (as expected) and the value is actually loaded and is seen by PTRACE_GETREGS (not expected).

This case is fixed by forcing a fatal SIGSEGV on the process so that it immediately dies. fatal_sigsegv was added for this purpose. It was declared as noreturn, so in order to pursuade gcc that it actually does not return, I added a call to os_dump_core (and declared it noreturn) so that I get a core file if somehow the process survives.

All other calls in arch/um/os-Linux/skas/process.c got the same treatment, with failures causing the process to die instead of a kernel panic, with some exceptions.

userspace_tramp exits with status 1 if anything goes wrong there. That will cause start_userspace to return an error. copy_context_skas0 and map_stub_pages also now return errors instead of panicing. Callers of thes functions were changed to check for errors and do something appropriate. Usually that's to return an error to their callers. check_skas3_ptrace_faultinfo just exits since that's too early to do anything else.

save_registers, restore_registers, and init_registers now return status instead of panicing on failure, with their callers doing something appropriate.

There were also duplicate declarations of save_registers and restore_registers in os.h - these are gone.

I noticed and fixed up some whitespace damage.

Signed-off-by Jeff Dike <[email protected]>

---
 arch/um/include/kern_util.h     |    1 
 arch/um/include/os.h            |    8 -
 arch/um/include/registers.h     |    6 
 arch/um/kernel/skas/mmu.c       |    5 
 arch/um/kernel/skas/process.c   |   20 ++
 arch/um/kernel/trap.c           |   12 +
 arch/um/os-Linux/registers.c    |   21 +--
 arch/um/os-Linux/skas/process.c |  267 ++++++++++++++++++++++++++--------------
 arch/um/os-Linux/start_up.c     |    4 
 arch/um/sys-x86_64/syscalls.c   |    8 -
 10 files changed, 234 insertions(+), 118 deletions(-)

arch/um/include/os.h +3 -5

Index: linux-2.6.22/arch/um/include/os.h
===================================================================
@@ -238,7 +238,7 @@ extern int raw(int fd);
 extern void setup_machinename(char *machine_out);
 extern void setup_hostinfo(char *buf, int len);
 extern int setjmp_wrapper(void (*proc)(void *, void *), ...);
-extern void os_dump_core(void);
+extern void os_dump_core(void) __attribute__ ((noreturn));
 extern void os_log_info(char *fmt, va_list ap);
 
 /* time.c */

@@ -268,11 +268,9 @@ extern int protect(struct mm_id * mm_idp
 extern int is_skas_winch(int pid, int fd, void *data);
 extern int start_userspace(unsigned long stub_stack);
 extern int copy_context_skas0(unsigned long stack, int pid);
-extern void save_registers(int pid, struct uml_pt_regs *regs);
-extern void restore_registers(int pid, struct uml_pt_regs *regs);
 extern void userspace(struct uml_pt_regs *regs);
-extern void map_stub_pages(int fd, unsigned long code,
-			   unsigned long data, unsigned long stack);
+extern int map_stub_pages(int fd, unsigned long code, unsigned long data,
+			  unsigned long stack);
 extern void new_thread(void *stack, jmp_buf *buf, void (*handler)(void));
 extern void switch_threads(jmp_buf *me, jmp_buf *you);
 extern int start_idle_thread(void *stack, jmp_buf *switch_buf);

arch/um/include/registers.h +3 -3

Index: linux-2.6.22/arch/um/include/registers.h
===================================================================
@@ -13,9 +13,9 @@ extern int save_fp_registers(int pid, un
 extern int restore_fp_registers(int pid, unsigned long *fp_regs);
 extern int save_fpx_registers(int pid, unsigned long *fp_regs);
 extern int restore_fpx_registers(int pid, unsigned long *fp_regs);
-extern void save_registers(int pid, struct uml_pt_regs *regs);
-extern void restore_registers(int pid, struct uml_pt_regs *regs);
-extern void init_registers(int pid);
+extern int save_registers(int pid, struct uml_pt_regs *regs);
+extern int restore_registers(int pid, struct uml_pt_regs *regs);
+extern int init_registers(int pid);
 extern void get_safe_registers(unsigned long *regs);
 extern unsigned long get_thread_reg(int reg, jmp_buf *buf);

arch/um/kernel/trap.c +12 -0

Index: linux-2.6.22/arch/um/kernel/trap.c
===================================================================
@@ -129,6 +129,18 @@ static void bad_segv(struct faultinfo fi
 	force_sig_info(SIGSEGV, &si, current);
 }
 
+void fatal_sigsegv(void)
+{
+	force_sigsegv(SIGSEGV, current);
+	do_signal();
+	/*
+	 * This is to tell gcc that we're not returning - do_signal
+	 * can, in general, return, but in this case, it's not, since
+	 * we just got a fatal SIGSEGV queued.
+	 */
+	os_dump_core();
+}
+
 void segv_handler(int sig, struct uml_pt_regs *regs)
 {
 	struct faultinfo * fi = UPT_FAULTINFO(regs);

arch/um/os-Linux/registers.c +10 ~~-11~~

Index: linux-2.6.22/arch/um/os-Linux/registers.c
===================================================================
@@ -8,42 +8,41 @@
 #include <string.h>
 #include <sys/ptrace.h>
 #include "sysdep/ptrace.h"
-#include "user.h"
 
-void save_registers(int pid, struct uml_pt_regs *regs)
+int save_registers(int pid, struct uml_pt_regs *regs)
 {
 	int err;
 
 	err = ptrace(PTRACE_GETREGS, pid, 0, regs->gp);
 	if (err < 0)
-		panic("save_registers - saving registers failed, errno = %d\n",
-		      errno);
+		return -errno;
+	return 0;
 }
 
-void restore_registers(int pid, struct uml_pt_regs *regs)
+int restore_registers(int pid, struct uml_pt_regs *regs)
 {
 	int err;
 
 	err = ptrace(PTRACE_SETREGS, pid, 0, regs->gp);
 	if (err < 0)
-		panic("restore_registers - saving registers failed, "
-		      "errno = %d\n", errno);
+		return -errno;
+	return 0;
 }
 
 /* This is set once at boot time and not changed thereafter */
 
 static unsigned long exec_regs[MAX_REG_NR];
 
-void init_registers(int pid)
+int init_registers(int pid)
 {
 	int err;
 
 	err = ptrace(PTRACE_GETREGS, pid, 0, exec_regs);
-	if (err)
-		panic("check_ptrace : PTRACE_GETREGS failed, errno = %d",
-		      errno);
+	if (err < 0)
+		return -errno;
 
 	arch_init_registers(pid);
+	return 0;
 }
 
 void get_safe_registers(unsigned long *regs)

arch/um/os-Linux/skas/process.c +175 ~~-90~~

430 lines not on this page · raw message

arch/um/include/kern_util.h +1 -0

Index: linux-2.6.22/arch/um/include/kern_util.h
===================================================================
@@ -62,6 +62,7 @@ extern int singlestepping(void *t);
 extern void segv_handler(int sig, struct uml_pt_regs *regs);
 extern void bus_handler(int sig, struct uml_pt_regs *regs);
 extern void winch(int sig, struct uml_pt_regs *regs);
+extern void fatal_sigsegv(void) __attribute__ ((noreturn));
 
 extern void log_info(char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
 extern void log_info_nolock(char *fmt, ...)

arch/um/os-Linux/start_up.c +3 -1

Index: linux-2.6.22/arch/um/os-Linux/start_up.c
===================================================================
@@ -411,7 +411,9 @@ static inline void check_skas3_ptrace_fa
 			non_fatal("found\n");
 	}
 
-	init_registers(pid);
+	if (init_registers(pid))
+		fatal("Failed to initialize default registers");
+
 	stop_ptraced_child(pid, 1, 1);
 }

arch/um/sys-x86_64/syscalls.c +5 -3

Index: linux-2.6.22/arch/um/sys-x86_64/syscalls.c
===================================================================
@@ -48,7 +48,9 @@ long arch_prctl(struct task_struct *task
 	switch (code) {
 	case ARCH_SET_FS:
 	case ARCH_SET_GS:
-		restore_registers(pid, &current->thread.regs.regs);
+		ret = restore_registers(pid, &current->thread.regs.regs);
+		if (ret)
+			return ret;
 		break;
 	case ARCH_GET_FS:
 	case ARCH_GET_GS:

@@ -70,10 +72,10 @@ long arch_prctl(struct task_struct *task
 	switch (code) {
 	case ARCH_SET_FS:
 		current->thread.arch.fs = (unsigned long) ptr;
-		save_registers(pid, &current->thread.regs.regs);
+		ret = save_registers(pid, &current->thread.regs.regs);
 		break;
 	case ARCH_SET_GS:
-		save_registers(pid, &current->thread.regs.regs);
+		ret = save_registers(pid, &current->thread.regs.regs);
 		break;
 	case ARCH_GET_FS:
 		ret = put_user(tmp, addr);

arch/um/kernel/skas/mmu.c +5 -0

Index: linux-2.6.22/arch/um/kernel/skas/mmu.c
===================================================================
@@ -86,6 +86,11 @@ int init_new_context(struct task_struct 
 			to_mm->id.u.pid = copy_context_skas0(stack,
 							     from_mm->id.u.pid);
 		else to_mm->id.u.pid = start_userspace(stack);
+
+		if (to_mm->id.u.pid < 0) {
+			ret = to_mm->id.u.pid;
+			goto out_free;
+		}
 	}
 
 	ret = init_new_ldt(to_mm, from_mm);

arch/um/kernel/skas/process.c +16 -4

Index: linux-2.6.22/arch/um/kernel/skas/process.c
===================================================================
@@ -6,19 +6,25 @@
 #include "linux/init.h"
 #include "linux/sched.h"
 #include "as-layout.h"
+#include "kern.h"
 #include "os.h"
 #include "skas.h"
 
 int new_mm(unsigned long stack)
 {
-	int fd;
+	int fd, err;
 
 	fd = os_open_file("/proc/mm", of_cloexec(of_write(OPENFLAGS())), 0);
 	if (fd < 0)
 		return fd;
 
-	if (skas_needs_stub)
-		map_stub_pages(fd, STUB_CODE, STUB_DATA, stack);
+	if (skas_needs_stub) {
+		err = map_stub_pages(fd, STUB_CODE, STUB_DATA, stack);
+		if (err) {
+			os_close_file(fd);
+			return err;
+		}
+	}
 
 	return fd;
 }

@@ -49,8 +55,14 @@ int __init start_uml(void)
 {
 	stack_protections((unsigned long) &cpu0_irqstack);
 	set_sigstack(cpu0_irqstack, THREAD_SIZE);
-	if (proc_mm)
+	if (proc_mm) {
 		userspace_pid[0] = start_userspace(0);
+		if (userspace_pid[0] < 0) {
+			printf("start_uml - start_userspace returned %d\n",
+			       userspace_pid[0]);
+			exit(1);
+		}
+	}
 
 	init_new_thread_signals();

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Andrew Morton

2007-11-20 21:26 UTC #1 lore raw

Quoted 1 line · On Tue, 20 Nov 2007 13:26:33 -0500 Jeff Dike <[email protected]> wrote:

+extern void os_dump_core(void) __attribute__ ((noreturn));

We have a __noreturn helper for this.

I'd have expected checkpatch to have a little whine about the space-before-(, but it didn't.