um: Fix ptrace GETREGS/SETREGS bugs
authorMickaël Salaün <mic@digikod.net>
Tue, 29 Dec 2015 20:35:44 +0000 (21:35 +0100)
committerRichard Weinberger <richard@nod.at>
Sun, 10 Jan 2016 20:49:48 +0000 (21:49 +0100)
This fix two related bugs:
* PTRACE_GETREGS doesn't get the right orig_ax (syscall) value
* PTRACE_SETREGS can't set the orig_ax value (erased by initial value)

Get rid of the now useless and error-prone get_syscall().

Fix inconsistent behavior in the ptrace implementation for i386 when
updating orig_eax automatically update the syscall number as well. This
is now updated in handle_syscall().

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Will Drewry <wad@chromium.org>
Cc: Thomas Meyer <thomas@m3y3r.de>
Cc: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
Cc: Anton Ivanov <aivanov@brocade.com>
Cc: Meredydd Luff <meredydd@senatehouse.org>
Cc: David Drysdale <drysdale@google.com>
Signed-off-by: Richard Weinberger <richard@nod.at>
Acked-by: Kees Cook <keescook@chromium.org>
arch/um/include/shared/os.h
arch/um/kernel/skas/syscall.c
arch/um/os-Linux/skas/process.c
arch/x86/um/ptrace_32.c

index 7a04ddd85334db17160fcbc2a97bba0396019523..de5d572225f3adb4c1cdab6e30c2dc3383ae02c1 100644 (file)
@@ -284,7 +284,6 @@ extern void initial_thread_cb_skas(void (*proc)(void *),
                                 void *arg);
 extern void halt_skas(void);
 extern void reboot_skas(void);
-extern int get_syscall(struct uml_pt_regs *regs);
 
 /* irq.c */
 extern int os_waiting_for_events(struct irq_fd *active_fds);
index 1683b8efdfdafe916c2768457e68f2dd5fd5b2d0..6cadce761bcf29c0dc170d97bfa4c9b8fed64de3 100644 (file)
@@ -7,29 +7,31 @@
 #include <linux/ptrace.h>
 #include <kern_util.h>
 #include <sysdep/ptrace.h>
+#include <sysdep/ptrace_user.h>
 #include <sysdep/syscalls.h>
-#include <os.h>
 
 void handle_syscall(struct uml_pt_regs *r)
 {
        struct pt_regs *regs = container_of(r, struct pt_regs, regs);
-       long result;
        int syscall;
 
-       if (syscall_trace_enter(regs)) {
-               result = -ENOSYS;
+       /* Initialize the syscall number and default return value. */
+       UPT_SYSCALL_NR(r) = PT_SYSCALL_NR(r->gp);
+       PT_REGS_SET_SYSCALL_RETURN(regs, -ENOSYS);
+
+       if (syscall_trace_enter(regs))
                goto out;
-       }
 
-       syscall = get_syscall(r);
+       /* Update the syscall number after orig_ax has potentially been updated
+        * with ptrace.
+        */
+       UPT_SYSCALL_NR(r) = PT_SYSCALL_NR(r->gp);
+       syscall = UPT_SYSCALL_NR(r);
 
-       if ((syscall > __NR_syscall_max) || syscall < 0)
-               result = -ENOSYS;
-       else
-               result = EXECUTE_SYSCALL(syscall, regs);
+       if (syscall >= 0 && syscall <= __NR_syscall_max)
+               PT_REGS_SET_SYSCALL_RETURN(regs,
+                               EXECUTE_SYSCALL(syscall, regs));
 
 out:
-       PT_REGS_SET_SYSCALL_RETURN(regs, result);
-
        syscall_trace_leave(regs);
 }
index b856c66ebd3a2be7c2a61027eb8ea616fe4cb1b9..23025d64516019e0d1451298570b5dc845b2bc62 100644 (file)
@@ -172,13 +172,6 @@ static void handle_trap(int pid, struct uml_pt_regs *regs,
        handle_syscall(regs);
 }
 
-int get_syscall(struct uml_pt_regs *regs)
-{
-       UPT_SYSCALL_NR(regs) = PT_SYSCALL_NR(regs->gp);
-
-       return UPT_SYSCALL_NR(regs);
-}
-
 extern char __syscall_stub_start[];
 
 static int userspace_tramp(void *stack)
index a29756f2d9402e45b7156aa609213663a935f671..47c78d5e5c3202a04f5c8a6a65432459142678f5 100644 (file)
@@ -68,6 +68,7 @@ static const int reg_offsets[] = {
        [EFL] = HOST_EFLAGS,
        [UESP] = HOST_SP,
        [SS] = HOST_SS,
+       [ORIG_EAX] = HOST_ORIG_AX,
 };
 
 int putreg(struct task_struct *child, int regno, unsigned long value)
@@ -83,6 +84,7 @@ int putreg(struct task_struct *child, int regno, unsigned long value)
        case EAX:
        case EIP:
        case UESP:
+       case ORIG_EAX:
                break;
        case FS:
                if (value && (value & 3) != 3)
@@ -108,9 +110,6 @@ int putreg(struct task_struct *child, int regno, unsigned long value)
                value &= FLAG_MASK;
                child->thread.regs.regs.gp[HOST_EFLAGS] |= value;
                return 0;
-       case ORIG_EAX:
-               child->thread.regs.regs.syscall = value;
-               return 0;
        default :
                panic("Bad register in putreg() : %d\n", regno);
        }
@@ -143,8 +142,6 @@ unsigned long getreg(struct task_struct *child, int regno)
 
        regno >>= 2;
        switch (regno) {
-       case ORIG_EAX:
-               return child->thread.regs.regs.syscall;
        case FS:
        case GS:
        case DS:
@@ -163,6 +160,7 @@ unsigned long getreg(struct task_struct *child, int regno)
        case EDI:
        case EBP:
        case EFL:
+       case ORIG_EAX:
                break;
        default:
                panic("Bad register in getreg() : %d\n", regno);