]>
Commit | Line | Data |
---|---|---|
321d628a FG |
1 | From 1e3688f9e76b3d8b218ed1afa292585a91b0b0c6 Mon Sep 17 00:00:00 2001 |
2 | From: Dave Hansen <dave.hansen@linux.intel.com> | |
3 | Date: Wed, 18 Oct 2017 10:21:07 -0700 | |
633c5ed1 | 4 | Subject: [PATCH 073/242] x86/entry: Use SYSCALL_DEFINE() macros for |
321d628a FG |
5 | sys_modify_ldt() |
6 | MIME-Version: 1.0 | |
7 | Content-Type: text/plain; charset=UTF-8 | |
8 | Content-Transfer-Encoding: 8bit | |
9 | ||
10 | CVE-2017-5754 | |
11 | ||
12 | We do not have tracepoints for sys_modify_ldt() because we define | |
13 | it directly instead of using the normal SYSCALL_DEFINEx() macros. | |
14 | ||
15 | However, there is a reason sys_modify_ldt() does not use the macros: | |
16 | it has an 'int' return type instead of 'unsigned long'. This is | |
17 | a bug, but it's a bug cemented in the ABI. | |
18 | ||
19 | What does this mean? If we return -EINVAL from a function that | |
20 | returns 'int', we have 0x00000000ffffffea in %rax. But, if we | |
21 | return -EINVAL from a function returning 'unsigned long', we end | |
22 | up with 0xffffffffffffffea in %rax, which is wrong. | |
23 | ||
24 | To work around this and maintain the 'int' behavior while using | |
25 | the SYSCALL_DEFINEx() macros, so we add a cast to 'unsigned int' | |
26 | in both implementations of sys_modify_ldt(). | |
27 | ||
28 | Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com> | |
29 | Reviewed-by: Andy Lutomirski <luto@kernel.org> | |
30 | Reviewed-by: Brian Gerst <brgerst@gmail.com> | |
31 | Cc: Linus Torvalds <torvalds@linux-foundation.org> | |
32 | Cc: Peter Zijlstra <peterz@infradead.org> | |
33 | Cc: Thomas Gleixner <tglx@linutronix.de> | |
34 | Link: http://lkml.kernel.org/r/20171018172107.1A79C532@viggo.jf.intel.com | |
35 | Signed-off-by: Ingo Molnar <mingo@kernel.org> | |
36 | (cherry picked from commit da20ab35180780e4a6eadc804544f1fa967f3567) | |
37 | Signed-off-by: Andy Whitcroft <apw@canonical.com> | |
38 | Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> | |
39 | (cherry picked from commit d865f635f4b2c3307e79de9be5c49ea8bd4c43a6) | |
40 | Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> | |
41 | --- | |
42 | arch/x86/include/asm/syscalls.h | 2 +- | |
43 | arch/x86/kernel/ldt.c | 16 +++++++++++++--- | |
44 | arch/x86/um/ldt.c | 7 +++++-- | |
45 | 3 files changed, 19 insertions(+), 6 deletions(-) | |
46 | ||
47 | diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h | |
48 | index 91dfcafe27a6..bad25bb80679 100644 | |
49 | --- a/arch/x86/include/asm/syscalls.h | |
50 | +++ b/arch/x86/include/asm/syscalls.h | |
51 | @@ -21,7 +21,7 @@ asmlinkage long sys_ioperm(unsigned long, unsigned long, int); | |
52 | asmlinkage long sys_iopl(unsigned int); | |
53 | ||
54 | /* kernel/ldt.c */ | |
55 | -asmlinkage int sys_modify_ldt(int, void __user *, unsigned long); | |
56 | +asmlinkage long sys_modify_ldt(int, void __user *, unsigned long); | |
57 | ||
58 | /* kernel/signal.c */ | |
59 | asmlinkage long sys_rt_sigreturn(void); | |
60 | diff --git a/arch/x86/kernel/ldt.c b/arch/x86/kernel/ldt.c | |
61 | index f0e64db18ac8..0402d44deb4d 100644 | |
62 | --- a/arch/x86/kernel/ldt.c | |
63 | +++ b/arch/x86/kernel/ldt.c | |
64 | @@ -12,6 +12,7 @@ | |
65 | #include <linux/string.h> | |
66 | #include <linux/mm.h> | |
67 | #include <linux/smp.h> | |
68 | +#include <linux/syscalls.h> | |
69 | #include <linux/slab.h> | |
70 | #include <linux/vmalloc.h> | |
71 | #include <linux/uaccess.h> | |
72 | @@ -294,8 +295,8 @@ static int write_ldt(void __user *ptr, unsigned long bytecount, int oldmode) | |
73 | return error; | |
74 | } | |
75 | ||
76 | -asmlinkage int sys_modify_ldt(int func, void __user *ptr, | |
77 | - unsigned long bytecount) | |
78 | +SYSCALL_DEFINE3(modify_ldt, int , func , void __user * , ptr , | |
79 | + unsigned long , bytecount) | |
80 | { | |
81 | int ret = -ENOSYS; | |
82 | ||
83 | @@ -313,5 +314,14 @@ asmlinkage int sys_modify_ldt(int func, void __user *ptr, | |
84 | ret = write_ldt(ptr, bytecount, 0); | |
85 | break; | |
86 | } | |
87 | - return ret; | |
88 | + /* | |
89 | + * The SYSCALL_DEFINE() macros give us an 'unsigned long' | |
90 | + * return type, but tht ABI for sys_modify_ldt() expects | |
91 | + * 'int'. This cast gives us an int-sized value in %rax | |
92 | + * for the return code. The 'unsigned' is necessary so | |
93 | + * the compiler does not try to sign-extend the negative | |
94 | + * return codes into the high half of the register when | |
95 | + * taking the value from int->long. | |
96 | + */ | |
97 | + return (unsigned int)ret; | |
98 | } | |
99 | diff --git a/arch/x86/um/ldt.c b/arch/x86/um/ldt.c | |
100 | index 836a1eb5df43..3ee234b6234d 100644 | |
101 | --- a/arch/x86/um/ldt.c | |
102 | +++ b/arch/x86/um/ldt.c | |
103 | @@ -6,6 +6,7 @@ | |
104 | #include <linux/mm.h> | |
105 | #include <linux/sched.h> | |
106 | #include <linux/slab.h> | |
107 | +#include <linux/syscalls.h> | |
108 | #include <linux/uaccess.h> | |
109 | #include <asm/unistd.h> | |
110 | #include <os.h> | |
111 | @@ -369,7 +370,9 @@ void free_ldt(struct mm_context *mm) | |
112 | mm->arch.ldt.entry_count = 0; | |
113 | } | |
114 | ||
115 | -int sys_modify_ldt(int func, void __user *ptr, unsigned long bytecount) | |
116 | +SYSCALL_DEFINE3(modify_ldt, int , func , void __user * , ptr , | |
117 | + unsigned long , bytecount) | |
118 | { | |
119 | - return do_modify_ldt_skas(func, ptr, bytecount); | |
120 | + /* See non-um modify_ldt() for why we do this cast */ | |
121 | + return (unsigned int)do_modify_ldt_skas(func, ptr, bytecount); | |
122 | } | |
123 | -- | |
124 | 2.14.2 | |
125 |