Define a weak function in COND_SYSCALL instead of a weak alias to
sys_ni_syscall, which has an incompatible type. This fixes indirect
call mismatches with Control-Flow Integrity (CFI) checking.
Signed-off-by: Sami Tolvanen <[email protected]>
---
arch/x86/include/asm/syscall_wrapper.h | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h
index 90eb70df0b18..9a595a544017 100644
--- a/arch/x86/include/asm/syscall_wrapper.h
+++ b/arch/x86/include/asm/syscall_wrapper.h
@@ -6,6 +6,8 @@
#ifndef _ASM_X86_SYSCALL_WRAPPER_H
#define _ASM_X86_SYSCALL_WRAPPER_H
+struct pt_regs;
+
/* Mapping of registers to parameters for syscalls on x86-64 and x32 */
#define SC_X86_64_REGS_TO_ARGS(x, ...) \
__MAP(x,__SC_ARGS \
@@ -56,9 +58,15 @@
SYSCALL_ALIAS(__ia32_sys_##sname, __x64_sys_##sname); \
asmlinkage long __x64_sys_##sname(const struct pt_regs *__unused)
-#define COND_SYSCALL(name) \
- cond_syscall(__x64_sys_##name); \
- cond_syscall(__ia32_sys_##name)
+#define COND_SYSCALL(name) \
+ asmlinkage __weak long __x64_sys_##name(const struct pt_regs *__unused) \
+ { \
+ return sys_ni_syscall(); \
+ } \
+ asmlinkage __weak long __ia32_sys_##name(const struct pt_regs *__unused)\
+ { \
+ return sys_ni_syscall(); \
+ }
#define SYS_NI(name) \
SYSCALL_ALIAS(__x64_sys_##name, sys_ni_posix_timers); \
@@ -190,7 +198,11 @@
#endif
#ifndef COND_SYSCALL
-#define COND_SYSCALL(name) cond_syscall(__x64_sys_##name)
+#define COND_SYSCALL(name) \
+ asmlinkage __weak long __x64_sys_##name(const struct pt_regs *__unused) \
+ { \
+ return sys_ni_syscall(); \
+ }
#endif
#ifndef SYS_NI
@@ -202,7 +214,6 @@
* For VSYSCALLS, we need to declare these three syscalls with the new
* pt_regs-based calling convention for in-kernel use.
*/
-struct pt_regs;
asmlinkage long __x64_sys_getcpu(const struct pt_regs *regs);
asmlinkage long __x64_sys_gettimeofday(const struct pt_regs *regs);
asmlinkage long __x64_sys_time(const struct pt_regs *regs);
--
2.23.0.237.gc6a4ce50a0-goog
On Fri, Sep 13, 2019 at 2:00 PM Sami Tolvanen <[email protected]> wrote:
>
> Define a weak function in COND_SYSCALL instead of a weak alias to
> sys_ni_syscall, which has an incompatible type. This fixes indirect
> call mismatches with Control-Flow Integrity (CFI) checking.
>
Didn't you just fix the type of sys_ni_syscall? What am I missing here?
On Fri, Sep 13, 2019 at 3:46 PM Andy Lutomirski <[email protected]> wrote:
> Didn't you just fix the type of sys_ni_syscall? What am I missing here?
The other patch fixes indirect call type mismatches when the function
is called through the syscall table. However, cond_syscall creates an
alias to the actual sys_ni_syscall function defined in
kernel/sys_ni.c, which still has the wrong type.
Sami
> On Sep 13, 2019, at 4:28 PM, Sami Tolvanen <[email protected]> wrote:
>
>> On Fri, Sep 13, 2019 at 3:46 PM Andy Lutomirski <[email protected]> wrote:
>> Didn't you just fix the type of sys_ni_syscall? What am I missing here?
>
> The other patch fixes indirect call type mismatches when the function
> is called through the syscall table. However, cond_syscall creates an
> alias to the actual sys_ni_syscall function defined in
> kernel/sys_ni.c, which still has the wrong type.
>
Ah, I get it. Doesn’t this cause a little bit of code bloat, though? What if you made __x86_ni_syscall, etc (possibly using the *DEFINE_SYSCALL0 macros) and then generate weak aliases to those?
On Fri, Sep 13, 2019 at 5:28 PM Andy Lutomirski <[email protected]> wrote:
> Ah, I get it. Doesn’t this cause a little bit of code bloat, though?
A little bit yes, a few extra functions for syscalls that are not
otherwise implemented.
> What if you made __x86_ni_syscall, etc (possibly using the *DEFINE_SYSCALL0 macros) and then generate weak aliases to those?
That would be convenient, but COND_SYSCALL is used in kernel/sys_ni.c,
and we can't create an alias to a function defined elsewhere:
$ cat test.c
long b(void);
long a(void) __attribute__((alias("b")));
$ gcc -c test.c
test.c:2:6: error: ‘a’ aliased to undefined symbol ‘b’
long a(void) __attribute__((alias("b")));
^
Curiously, when we use inline assembly to create the alias (similarly
to the current cond_syscall), gcc just quietly drops the alias if the
function is not defined.
Sami