[PATCH] uml: more carefully test whether we are in a system call

For security reasons, UML in is_syscall() needs to have access to code in
vsyscall-page.  The current implementation grants this access by explicitly
allowing access to vsyscall in access_ok_skas().  With this change,
copy_from_user() may be used to read the code.  Ptrace access to vsyscall-page
for debugging already was implemented in get_user_pages() by mainline.  In
i386, copy_from_user can't access vsyscall-page, but returns EFAULT.

To make UML behave as i386 does, I changed is_syscall to use
access_process_vm(current) to read the code from vsyscall-page.  This doesn't
hurt security, but simplifies the code and prepares implementation of
stub-vmas.

Signed-off-by: Bodo Stroesser <bstroesser@fujitsu-siemens.com>
Signed-off-by: Jeff Dike <jdike@addtoit.com>
Cc: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/arch/um/sys-i386/ptrace.c b/arch/um/sys-i386/ptrace.c
index e839ce65ad287d2bfa172f58d6c2ceaae302e373..8032a105949a02155945d52e00e1123f29039a1e 100644 (file)
--- a/arch/um/sys-i386/ptrace.c
+++ b/arch/um/sys-i386/ptrace.c
@@ -6,6 +6,7 @@
 #include <linux/config.h>
 #include <linux/compiler.h>
 #include "linux/sched.h"
+#include "linux/mm.h"
 #include "asm/elf.h"
 #include "asm/ptrace.h"
 #include "asm/uaccess.h"
@@ -26,9 +27,17 @@ int is_syscall(unsigned long addr)
 
        n = copy_from_user(&instr, (void __user *) addr, sizeof(instr));
        if(n){
-               printk("is_syscall : failed to read instruction from 0x%lx\n",
-                      addr);
-               return(0);
+               /* access_process_vm() grants access to vsyscall and stub,
+                * while copy_from_user doesn't. Maybe access_process_vm is
+                * slow, but that doesn't matter, since it will be called only
+                * in case of singlestepping, if copy_from_user failed.
+                */
+               n = access_process_vm(current, addr, &instr, sizeof(instr), 0);
+               if(n != sizeof(instr)) {
+                       printk("is_syscall : failed to read instruction from "
+                              "0x%lx\n", addr);
+                       return(1);
+               }
        }
        /* int 0x80 or sysenter */
        return((instr == 0x80cd) || (instr == 0x340f));

diff --git a/arch/um/sys-x86_64/ptrace.c b/arch/um/sys-x86_64/ptrace.c
index 74eee5c7c6dd7aa3b6b79d244bb54d2cda33439a..147bbf05cbc2fbc2d71e395f038ac97ce0387902 100644 (file)
--- a/arch/um/sys-x86_64/ptrace.c
+++ b/arch/um/sys-x86_64/ptrace.c
@@ -8,6 +8,7 @@
 #include <asm/ptrace.h>
 #include <linux/sched.h>
 #include <linux/errno.h>
+#include <linux/mm.h>
 #include <asm/uaccess.h>
 #include <asm/elf.h>
 
@@ -136,9 +137,28 @@ void arch_switch(void)
 */
 }
 
+/* XXX Mostly copied from sys-i386 */
 int is_syscall(unsigned long addr)
 {
-       panic("is_syscall");
+       unsigned short instr;
+       int n;
+
+       n = copy_from_user(&instr, (void __user *) addr, sizeof(instr));
+       if(n){
+               /* access_process_vm() grants access to vsyscall and stub,
+                * while copy_from_user doesn't. Maybe access_process_vm is
+                * slow, but that doesn't matter, since it will be called only
+                * in case of singlestepping, if copy_from_user failed.
+                */
+               n = access_process_vm(current, addr, &instr, sizeof(instr), 0);
+               if(n != sizeof(instr)) {
+                       printk("is_syscall : failed to read instruction from "
+                              "0x%lx\n", addr);
+                       return(1);
+               }
+       }
+       /* sysenter */
+       return(instr == 0x050f);
 }
 
 int dump_fpu(struct pt_regs *regs, elf_fpregset_t *fpu )