* [PATCH v2 0/3] fix function type mismatches in syscall wrappers @ 2019-05-03 19:12 ` Sami Tolvanen 0 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-03 19:12 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland Cc: Kees Cook, Nick Desaulniers, linux-arm-kernel, linux-kernel, Sami Tolvanen These patches fix type mismatches in arm64 syscall wrapper definitions, which trip indirect call checks with Control-Flow Integrity. Changes in v2: - more informative commit message for the syscall_fn_t change - added a patch for fixing sys_ni_syscall Sami Tolvanen (3): arm64: fix syscall_fn_t type arm64: use the correct function type in SYSCALL_DEFINE0 arm64: use the correct function type for __arm64_sys_ni_syscall arch/arm64/include/asm/syscall.h | 2 +- arch/arm64/include/asm/syscall_wrapper.h | 18 +++++++++--------- arch/arm64/kernel/sys.c | 14 +++++++++----- arch/arm64/kernel/sys32.c | 12 ++++++++---- 4 files changed, 27 insertions(+), 19 deletions(-) -- 2.21.0.1020.gf2820cf01a-goog ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/3] fix function type mismatches in syscall wrappers @ 2019-05-03 19:12 ` Sami Tolvanen 0 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-03 19:12 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland Cc: Sami Tolvanen, Nick Desaulniers, Kees Cook, linux-arm-kernel, linux-kernel These patches fix type mismatches in arm64 syscall wrapper definitions, which trip indirect call checks with Control-Flow Integrity. Changes in v2: - more informative commit message for the syscall_fn_t change - added a patch for fixing sys_ni_syscall Sami Tolvanen (3): arm64: fix syscall_fn_t type arm64: use the correct function type in SYSCALL_DEFINE0 arm64: use the correct function type for __arm64_sys_ni_syscall arch/arm64/include/asm/syscall.h | 2 +- arch/arm64/include/asm/syscall_wrapper.h | 18 +++++++++--------- arch/arm64/kernel/sys.c | 14 +++++++++----- arch/arm64/kernel/sys32.c | 12 ++++++++---- 4 files changed, 27 insertions(+), 19 deletions(-) -- 2.21.0.1020.gf2820cf01a-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/3] arm64: fix syscall_fn_t type 2019-05-03 19:12 ` Sami Tolvanen @ 2019-05-03 19:12 ` Sami Tolvanen -1 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-03 19:12 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland Cc: Kees Cook, Nick Desaulniers, linux-arm-kernel, linux-kernel, Sami Tolvanen Syscall wrappers in <asm/syscall_wrapper.h> use const struct pt_regs * as the argument type. Use const in syscall_fn_t as well to fix indirect call type mismatches with Control-Flow Integrity checking. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> Reviewed-by: Mark Rutland <mark.rutland@arm.com> --- arch/arm64/include/asm/syscall.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h index a179df3674a1a..6206ab9bfcfc5 100644 --- a/arch/arm64/include/asm/syscall.h +++ b/arch/arm64/include/asm/syscall.h @@ -20,7 +20,7 @@ #include <linux/compat.h> #include <linux/err.h> -typedef long (*syscall_fn_t)(struct pt_regs *regs); +typedef long (*syscall_fn_t)(const struct pt_regs *regs); extern const syscall_fn_t sys_call_table[]; -- 2.21.0.1020.gf2820cf01a-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 1/3] arm64: fix syscall_fn_t type @ 2019-05-03 19:12 ` Sami Tolvanen 0 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-03 19:12 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland Cc: Sami Tolvanen, Nick Desaulniers, Kees Cook, linux-arm-kernel, linux-kernel Syscall wrappers in <asm/syscall_wrapper.h> use const struct pt_regs * as the argument type. Use const in syscall_fn_t as well to fix indirect call type mismatches with Control-Flow Integrity checking. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> Reviewed-by: Mark Rutland <mark.rutland@arm.com> --- arch/arm64/include/asm/syscall.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h index a179df3674a1a..6206ab9bfcfc5 100644 --- a/arch/arm64/include/asm/syscall.h +++ b/arch/arm64/include/asm/syscall.h @@ -20,7 +20,7 @@ #include <linux/compat.h> #include <linux/err.h> -typedef long (*syscall_fn_t)(struct pt_regs *regs); +typedef long (*syscall_fn_t)(const struct pt_regs *regs); extern const syscall_fn_t sys_call_table[]; -- 2.21.0.1020.gf2820cf01a-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/3] arm64: use the correct function type in SYSCALL_DEFINE0 2019-05-03 19:12 ` Sami Tolvanen @ 2019-05-03 19:12 ` Sami Tolvanen -1 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-03 19:12 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland Cc: Kees Cook, Nick Desaulniers, linux-arm-kernel, linux-kernel, Sami Tolvanen Although a syscall defined using SYSCALL_DEFINE0 doesn't accept parameters, use the correct function type to avoid indirect call type mismatches with Control-Flow Integrity checking. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- arch/arm64/include/asm/syscall_wrapper.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index a4477e515b798..507d0ee6bc690 100644 --- a/arch/arm64/include/asm/syscall_wrapper.h +++ b/arch/arm64/include/asm/syscall_wrapper.h @@ -30,10 +30,10 @@ } \ static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) -#define COMPAT_SYSCALL_DEFINE0(sname) \ - asmlinkage long __arm64_compat_sys_##sname(void); \ - ALLOW_ERROR_INJECTION(__arm64_compat_sys_##sname, ERRNO); \ - asmlinkage long __arm64_compat_sys_##sname(void) +#define COMPAT_SYSCALL_DEFINE0(sname) \ + asmlinkage long __arm64_compat_sys_##sname(const struct pt_regs *__unused); \ + ALLOW_ERROR_INJECTION(__arm64_compat_sys_##sname, ERRNO); \ + asmlinkage long __arm64_compat_sys_##sname(const struct pt_regs *__unused) #define COND_SYSCALL_COMPAT(name) \ cond_syscall(__arm64_compat_sys_##name); @@ -62,11 +62,11 @@ static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) #ifndef SYSCALL_DEFINE0 -#define SYSCALL_DEFINE0(sname) \ - SYSCALL_METADATA(_##sname, 0); \ - asmlinkage long __arm64_sys_##sname(void); \ - ALLOW_ERROR_INJECTION(__arm64_sys_##sname, ERRNO); \ - asmlinkage long __arm64_sys_##sname(void) +#define SYSCALL_DEFINE0(sname) \ + SYSCALL_METADATA(_##sname, 0); \ + asmlinkage long __arm64_sys_##sname(const struct pt_regs *__unused); \ + ALLOW_ERROR_INJECTION(__arm64_sys_##sname, ERRNO); \ + asmlinkage long __arm64_sys_##sname(const struct pt_regs *__unused) #endif #ifndef COND_SYSCALL -- 2.21.0.1020.gf2820cf01a-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/3] arm64: use the correct function type in SYSCALL_DEFINE0 @ 2019-05-03 19:12 ` Sami Tolvanen 0 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-03 19:12 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland Cc: Sami Tolvanen, Nick Desaulniers, Kees Cook, linux-arm-kernel, linux-kernel Although a syscall defined using SYSCALL_DEFINE0 doesn't accept parameters, use the correct function type to avoid indirect call type mismatches with Control-Flow Integrity checking. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- arch/arm64/include/asm/syscall_wrapper.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index a4477e515b798..507d0ee6bc690 100644 --- a/arch/arm64/include/asm/syscall_wrapper.h +++ b/arch/arm64/include/asm/syscall_wrapper.h @@ -30,10 +30,10 @@ } \ static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) -#define COMPAT_SYSCALL_DEFINE0(sname) \ - asmlinkage long __arm64_compat_sys_##sname(void); \ - ALLOW_ERROR_INJECTION(__arm64_compat_sys_##sname, ERRNO); \ - asmlinkage long __arm64_compat_sys_##sname(void) +#define COMPAT_SYSCALL_DEFINE0(sname) \ + asmlinkage long __arm64_compat_sys_##sname(const struct pt_regs *__unused); \ + ALLOW_ERROR_INJECTION(__arm64_compat_sys_##sname, ERRNO); \ + asmlinkage long __arm64_compat_sys_##sname(const struct pt_regs *__unused) #define COND_SYSCALL_COMPAT(name) \ cond_syscall(__arm64_compat_sys_##name); @@ -62,11 +62,11 @@ static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) #ifndef SYSCALL_DEFINE0 -#define SYSCALL_DEFINE0(sname) \ - SYSCALL_METADATA(_##sname, 0); \ - asmlinkage long __arm64_sys_##sname(void); \ - ALLOW_ERROR_INJECTION(__arm64_sys_##sname, ERRNO); \ - asmlinkage long __arm64_sys_##sname(void) +#define SYSCALL_DEFINE0(sname) \ + SYSCALL_METADATA(_##sname, 0); \ + asmlinkage long __arm64_sys_##sname(const struct pt_regs *__unused); \ + ALLOW_ERROR_INJECTION(__arm64_sys_##sname, ERRNO); \ + asmlinkage long __arm64_sys_##sname(const struct pt_regs *__unused) #endif #ifndef COND_SYSCALL -- 2.21.0.1020.gf2820cf01a-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall 2019-05-03 19:12 ` Sami Tolvanen @ 2019-05-03 19:12 ` Sami Tolvanen -1 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-03 19:12 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland Cc: Kees Cook, Nick Desaulniers, linux-arm-kernel, linux-kernel, Sami Tolvanen Calling sys_ni_syscall through a syscall_fn_t pointer trips indirect call Control-Flow Integrity checking due to a function type mismatch. Use SYSCALL_DEFINE0 for __arm64_sys_ni_syscall instead and remove the now unnecessary casts. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- arch/arm64/kernel/sys.c | 14 +++++++++----- arch/arm64/kernel/sys32.c | 12 ++++++++---- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c index b44065fb16160..4f8e8a7237a85 100644 --- a/arch/arm64/kernel/sys.c +++ b/arch/arm64/kernel/sys.c @@ -47,22 +47,26 @@ SYSCALL_DEFINE1(arm64_personality, unsigned int, personality) return ksys_personality(personality); } +asmlinkage long sys_ni_syscall(void); + +SYSCALL_DEFINE0(ni_syscall) +{ + return sys_ni_syscall(); +} + /* * Wrappers to pass the pt_regs argument. */ #define sys_personality sys_arm64_personality -asmlinkage long sys_ni_syscall(const struct pt_regs *); -#define __arm64_sys_ni_syscall sys_ni_syscall - #undef __SYSCALL #define __SYSCALL(nr, sym) asmlinkage long __arm64_##sym(const struct pt_regs *); #include <asm/unistd.h> #undef __SYSCALL -#define __SYSCALL(nr, sym) [nr] = (syscall_fn_t)__arm64_##sym, +#define __SYSCALL(nr, sym) [nr] = __arm64_##sym, const syscall_fn_t sys_call_table[__NR_syscalls] = { - [0 ... __NR_syscalls - 1] = (syscall_fn_t)sys_ni_syscall, + [0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall, #include <asm/unistd.h> }; diff --git a/arch/arm64/kernel/sys32.c b/arch/arm64/kernel/sys32.c index 0f8bcb7de7008..f8f6c26cfd326 100644 --- a/arch/arm64/kernel/sys32.c +++ b/arch/arm64/kernel/sys32.c @@ -133,17 +133,21 @@ COMPAT_SYSCALL_DEFINE6(aarch32_fallocate, int, fd, int, mode, return ksys_fallocate(fd, mode, arg_u64(offset), arg_u64(len)); } -asmlinkage long sys_ni_syscall(const struct pt_regs *); -#define __arm64_sys_ni_syscall sys_ni_syscall +asmlinkage long sys_ni_syscall(void); + +COMPAT_SYSCALL_DEFINE0(ni_syscall) +{ + return sys_ni_syscall(); +} #undef __SYSCALL #define __SYSCALL(nr, sym) asmlinkage long __arm64_##sym(const struct pt_regs *); #include <asm/unistd32.h> #undef __SYSCALL -#define __SYSCALL(nr, sym) [nr] = (syscall_fn_t)__arm64_##sym, +#define __SYSCALL(nr, sym) [nr] = __arm64_##sym, const syscall_fn_t compat_sys_call_table[__NR_compat_syscalls] = { - [0 ... __NR_compat_syscalls - 1] = (syscall_fn_t)sys_ni_syscall, + [0 ... __NR_compat_syscalls - 1] = __arm64_sys_ni_syscall, #include <asm/unistd32.h> }; -- 2.21.0.1020.gf2820cf01a-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall @ 2019-05-03 19:12 ` Sami Tolvanen 0 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-03 19:12 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland Cc: Sami Tolvanen, Nick Desaulniers, Kees Cook, linux-arm-kernel, linux-kernel Calling sys_ni_syscall through a syscall_fn_t pointer trips indirect call Control-Flow Integrity checking due to a function type mismatch. Use SYSCALL_DEFINE0 for __arm64_sys_ni_syscall instead and remove the now unnecessary casts. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- arch/arm64/kernel/sys.c | 14 +++++++++----- arch/arm64/kernel/sys32.c | 12 ++++++++---- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c index b44065fb16160..4f8e8a7237a85 100644 --- a/arch/arm64/kernel/sys.c +++ b/arch/arm64/kernel/sys.c @@ -47,22 +47,26 @@ SYSCALL_DEFINE1(arm64_personality, unsigned int, personality) return ksys_personality(personality); } +asmlinkage long sys_ni_syscall(void); + +SYSCALL_DEFINE0(ni_syscall) +{ + return sys_ni_syscall(); +} + /* * Wrappers to pass the pt_regs argument. */ #define sys_personality sys_arm64_personality -asmlinkage long sys_ni_syscall(const struct pt_regs *); -#define __arm64_sys_ni_syscall sys_ni_syscall - #undef __SYSCALL #define __SYSCALL(nr, sym) asmlinkage long __arm64_##sym(const struct pt_regs *); #include <asm/unistd.h> #undef __SYSCALL -#define __SYSCALL(nr, sym) [nr] = (syscall_fn_t)__arm64_##sym, +#define __SYSCALL(nr, sym) [nr] = __arm64_##sym, const syscall_fn_t sys_call_table[__NR_syscalls] = { - [0 ... __NR_syscalls - 1] = (syscall_fn_t)sys_ni_syscall, + [0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall, #include <asm/unistd.h> }; diff --git a/arch/arm64/kernel/sys32.c b/arch/arm64/kernel/sys32.c index 0f8bcb7de7008..f8f6c26cfd326 100644 --- a/arch/arm64/kernel/sys32.c +++ b/arch/arm64/kernel/sys32.c @@ -133,17 +133,21 @@ COMPAT_SYSCALL_DEFINE6(aarch32_fallocate, int, fd, int, mode, return ksys_fallocate(fd, mode, arg_u64(offset), arg_u64(len)); } -asmlinkage long sys_ni_syscall(const struct pt_regs *); -#define __arm64_sys_ni_syscall sys_ni_syscall +asmlinkage long sys_ni_syscall(void); + +COMPAT_SYSCALL_DEFINE0(ni_syscall) +{ + return sys_ni_syscall(); +} #undef __SYSCALL #define __SYSCALL(nr, sym) asmlinkage long __arm64_##sym(const struct pt_regs *); #include <asm/unistd32.h> #undef __SYSCALL -#define __SYSCALL(nr, sym) [nr] = (syscall_fn_t)__arm64_##sym, +#define __SYSCALL(nr, sym) [nr] = __arm64_##sym, const syscall_fn_t compat_sys_call_table[__NR_compat_syscalls] = { - [0 ... __NR_compat_syscalls - 1] = (syscall_fn_t)sys_ni_syscall, + [0 ... __NR_compat_syscalls - 1] = __arm64_sys_ni_syscall, #include <asm/unistd32.h> }; -- 2.21.0.1020.gf2820cf01a-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall 2019-05-03 19:12 ` Sami Tolvanen @ 2019-05-07 17:25 ` Mark Rutland -1 siblings, 0 replies; 18+ messages in thread From: Mark Rutland @ 2019-05-07 17:25 UTC (permalink / raw) To: Sami Tolvanen Cc: Catalin Marinas, Will Deacon, Kees Cook, Nick Desaulniers, linux-arm-kernel, linux-kernel On Fri, May 03, 2019 at 12:12:25PM -0700, Sami Tolvanen wrote: > Calling sys_ni_syscall through a syscall_fn_t pointer trips indirect > call Control-Flow Integrity checking due to a function type > mismatch. Use SYSCALL_DEFINE0 for __arm64_sys_ni_syscall instead and > remove the now unnecessary casts. > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > --- > arch/arm64/kernel/sys.c | 14 +++++++++----- > arch/arm64/kernel/sys32.c | 12 ++++++++---- > 2 files changed, 17 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c > index b44065fb16160..4f8e8a7237a85 100644 > --- a/arch/arm64/kernel/sys.c > +++ b/arch/arm64/kernel/sys.c > @@ -47,22 +47,26 @@ SYSCALL_DEFINE1(arm64_personality, unsigned int, personality) > return ksys_personality(personality); > } > > +asmlinkage long sys_ni_syscall(void); > + > +SYSCALL_DEFINE0(ni_syscall) > +{ > + return sys_ni_syscall(); > +} I strongly think that we cant to fix up the common definition in kernel/sys_ni.c rather than having a point-hack in arm64. Other architectures (e.g. x86, s390) will want the same for CFI, and I'd like to ensure that our approached don't diverge. I took a quick look, and it looks like it's messy but possible to fix up the core. I also suspect that using SYSCALL_DEFINE0() as it currently stands isn't a great idea, since it'll allow fault injection for unimplemented syscalls, which sounds dubious to me. Thanks, Mark. > + > /* > * Wrappers to pass the pt_regs argument. > */ > #define sys_personality sys_arm64_personality > > -asmlinkage long sys_ni_syscall(const struct pt_regs *); > -#define __arm64_sys_ni_syscall sys_ni_syscall > - > #undef __SYSCALL > #define __SYSCALL(nr, sym) asmlinkage long __arm64_##sym(const struct pt_regs *); > #include <asm/unistd.h> > > #undef __SYSCALL > -#define __SYSCALL(nr, sym) [nr] = (syscall_fn_t)__arm64_##sym, > +#define __SYSCALL(nr, sym) [nr] = __arm64_##sym, > > const syscall_fn_t sys_call_table[__NR_syscalls] = { > - [0 ... __NR_syscalls - 1] = (syscall_fn_t)sys_ni_syscall, > + [0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall, > #include <asm/unistd.h> > }; > diff --git a/arch/arm64/kernel/sys32.c b/arch/arm64/kernel/sys32.c > index 0f8bcb7de7008..f8f6c26cfd326 100644 > --- a/arch/arm64/kernel/sys32.c > +++ b/arch/arm64/kernel/sys32.c > @@ -133,17 +133,21 @@ COMPAT_SYSCALL_DEFINE6(aarch32_fallocate, int, fd, int, mode, > return ksys_fallocate(fd, mode, arg_u64(offset), arg_u64(len)); > } > > -asmlinkage long sys_ni_syscall(const struct pt_regs *); > -#define __arm64_sys_ni_syscall sys_ni_syscall > +asmlinkage long sys_ni_syscall(void); > + > +COMPAT_SYSCALL_DEFINE0(ni_syscall) > +{ > + return sys_ni_syscall(); > +} > > #undef __SYSCALL > #define __SYSCALL(nr, sym) asmlinkage long __arm64_##sym(const struct pt_regs *); > #include <asm/unistd32.h> > > #undef __SYSCALL > -#define __SYSCALL(nr, sym) [nr] = (syscall_fn_t)__arm64_##sym, > +#define __SYSCALL(nr, sym) [nr] = __arm64_##sym, > > const syscall_fn_t compat_sys_call_table[__NR_compat_syscalls] = { > - [0 ... __NR_compat_syscalls - 1] = (syscall_fn_t)sys_ni_syscall, > + [0 ... __NR_compat_syscalls - 1] = __arm64_sys_ni_syscall, > #include <asm/unistd32.h> > }; > -- > 2.21.0.1020.gf2820cf01a-goog > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall @ 2019-05-07 17:25 ` Mark Rutland 0 siblings, 0 replies; 18+ messages in thread From: Mark Rutland @ 2019-05-07 17:25 UTC (permalink / raw) To: Sami Tolvanen Cc: Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel, Will Deacon, linux-arm-kernel On Fri, May 03, 2019 at 12:12:25PM -0700, Sami Tolvanen wrote: > Calling sys_ni_syscall through a syscall_fn_t pointer trips indirect > call Control-Flow Integrity checking due to a function type > mismatch. Use SYSCALL_DEFINE0 for __arm64_sys_ni_syscall instead and > remove the now unnecessary casts. > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > --- > arch/arm64/kernel/sys.c | 14 +++++++++----- > arch/arm64/kernel/sys32.c | 12 ++++++++---- > 2 files changed, 17 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c > index b44065fb16160..4f8e8a7237a85 100644 > --- a/arch/arm64/kernel/sys.c > +++ b/arch/arm64/kernel/sys.c > @@ -47,22 +47,26 @@ SYSCALL_DEFINE1(arm64_personality, unsigned int, personality) > return ksys_personality(personality); > } > > +asmlinkage long sys_ni_syscall(void); > + > +SYSCALL_DEFINE0(ni_syscall) > +{ > + return sys_ni_syscall(); > +} I strongly think that we cant to fix up the common definition in kernel/sys_ni.c rather than having a point-hack in arm64. Other architectures (e.g. x86, s390) will want the same for CFI, and I'd like to ensure that our approached don't diverge. I took a quick look, and it looks like it's messy but possible to fix up the core. I also suspect that using SYSCALL_DEFINE0() as it currently stands isn't a great idea, since it'll allow fault injection for unimplemented syscalls, which sounds dubious to me. Thanks, Mark. > + > /* > * Wrappers to pass the pt_regs argument. > */ > #define sys_personality sys_arm64_personality > > -asmlinkage long sys_ni_syscall(const struct pt_regs *); > -#define __arm64_sys_ni_syscall sys_ni_syscall > - > #undef __SYSCALL > #define __SYSCALL(nr, sym) asmlinkage long __arm64_##sym(const struct pt_regs *); > #include <asm/unistd.h> > > #undef __SYSCALL > -#define __SYSCALL(nr, sym) [nr] = (syscall_fn_t)__arm64_##sym, > +#define __SYSCALL(nr, sym) [nr] = __arm64_##sym, > > const syscall_fn_t sys_call_table[__NR_syscalls] = { > - [0 ... __NR_syscalls - 1] = (syscall_fn_t)sys_ni_syscall, > + [0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall, > #include <asm/unistd.h> > }; > diff --git a/arch/arm64/kernel/sys32.c b/arch/arm64/kernel/sys32.c > index 0f8bcb7de7008..f8f6c26cfd326 100644 > --- a/arch/arm64/kernel/sys32.c > +++ b/arch/arm64/kernel/sys32.c > @@ -133,17 +133,21 @@ COMPAT_SYSCALL_DEFINE6(aarch32_fallocate, int, fd, int, mode, > return ksys_fallocate(fd, mode, arg_u64(offset), arg_u64(len)); > } > > -asmlinkage long sys_ni_syscall(const struct pt_regs *); > -#define __arm64_sys_ni_syscall sys_ni_syscall > +asmlinkage long sys_ni_syscall(void); > + > +COMPAT_SYSCALL_DEFINE0(ni_syscall) > +{ > + return sys_ni_syscall(); > +} > > #undef __SYSCALL > #define __SYSCALL(nr, sym) asmlinkage long __arm64_##sym(const struct pt_regs *); > #include <asm/unistd32.h> > > #undef __SYSCALL > -#define __SYSCALL(nr, sym) [nr] = (syscall_fn_t)__arm64_##sym, > +#define __SYSCALL(nr, sym) [nr] = __arm64_##sym, > > const syscall_fn_t compat_sys_call_table[__NR_compat_syscalls] = { > - [0 ... __NR_compat_syscalls - 1] = (syscall_fn_t)sys_ni_syscall, > + [0 ... __NR_compat_syscalls - 1] = __arm64_sys_ni_syscall, > #include <asm/unistd32.h> > }; > -- > 2.21.0.1020.gf2820cf01a-goog > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall 2019-05-07 17:25 ` Mark Rutland @ 2019-05-07 18:32 ` Sami Tolvanen -1 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-07 18:32 UTC (permalink / raw) To: Mark Rutland Cc: Catalin Marinas, Will Deacon, Kees Cook, Nick Desaulniers, linux-arm-kernel, linux-kernel On Tue, May 07, 2019 at 06:25:12PM +0100, Mark Rutland wrote: > I strongly think that we cant to fix up the common definition in > kernel/sys_ni.c rather than having a point-hack in arm64. Other > architectures (e.g. x86, s390) will want the same for CFI, and I'd like > to ensure that our approached don't diverge. s390 already has the following in arch/s390/kernel/sys_s390.c: SYSCALL_DEFINE0(ni_syscall) { return -ENOSYS; } Which, I suppose, is cleaner than calling sys_ni_syscall. > I took a quick look, and it looks like it's messy but possible to fix > up the core. OK. How would you propose fixing this? Sami ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall @ 2019-05-07 18:32 ` Sami Tolvanen 0 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-07 18:32 UTC (permalink / raw) To: Mark Rutland Cc: Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel, Will Deacon, linux-arm-kernel On Tue, May 07, 2019 at 06:25:12PM +0100, Mark Rutland wrote: > I strongly think that we cant to fix up the common definition in > kernel/sys_ni.c rather than having a point-hack in arm64. Other > architectures (e.g. x86, s390) will want the same for CFI, and I'd like > to ensure that our approached don't diverge. s390 already has the following in arch/s390/kernel/sys_s390.c: SYSCALL_DEFINE0(ni_syscall) { return -ENOSYS; } Which, I suppose, is cleaner than calling sys_ni_syscall. > I took a quick look, and it looks like it's messy but possible to fix > up the core. OK. How would you propose fixing this? Sami _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall 2019-05-07 18:32 ` Sami Tolvanen @ 2019-05-15 11:40 ` Will Deacon -1 siblings, 0 replies; 18+ messages in thread From: Will Deacon @ 2019-05-15 11:40 UTC (permalink / raw) To: Sami Tolvanen Cc: Mark Rutland, Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel, linux-arm-kernel On Tue, May 07, 2019 at 11:32:27AM -0700, Sami Tolvanen wrote: > On Tue, May 07, 2019 at 06:25:12PM +0100, Mark Rutland wrote: > > I strongly think that we cant to fix up the common definition in > > kernel/sys_ni.c rather than having a point-hack in arm64. Other > > architectures (e.g. x86, s390) will want the same for CFI, and I'd like > > to ensure that our approached don't diverge. > > s390 already has the following in arch/s390/kernel/sys_s390.c: > > SYSCALL_DEFINE0(ni_syscall) > { > return -ENOSYS; > } > > Which, I suppose, is cleaner than calling sys_ni_syscall. > > > I took a quick look, and it looks like it's messy but possible to fix > > up the core. > > OK. How would you propose fixing this? In the absence of a patch from Mark, I'd suggest just adding a SYS_NI macro to our asm/syscall_wrapper.h file which avoids the error injection stuff. It doesn't preclude moving this to the core later on, but it unblocks the CFI work. Will ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall @ 2019-05-15 11:40 ` Will Deacon 0 siblings, 0 replies; 18+ messages in thread From: Will Deacon @ 2019-05-15 11:40 UTC (permalink / raw) To: Sami Tolvanen Cc: Mark Rutland, Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel, linux-arm-kernel On Tue, May 07, 2019 at 11:32:27AM -0700, Sami Tolvanen wrote: > On Tue, May 07, 2019 at 06:25:12PM +0100, Mark Rutland wrote: > > I strongly think that we cant to fix up the common definition in > > kernel/sys_ni.c rather than having a point-hack in arm64. Other > > architectures (e.g. x86, s390) will want the same for CFI, and I'd like > > to ensure that our approached don't diverge. > > s390 already has the following in arch/s390/kernel/sys_s390.c: > > SYSCALL_DEFINE0(ni_syscall) > { > return -ENOSYS; > } > > Which, I suppose, is cleaner than calling sys_ni_syscall. > > > I took a quick look, and it looks like it's messy but possible to fix > > up the core. > > OK. How would you propose fixing this? In the absence of a patch from Mark, I'd suggest just adding a SYS_NI macro to our asm/syscall_wrapper.h file which avoids the error injection stuff. It doesn't preclude moving this to the core later on, but it unblocks the CFI work. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall 2019-05-15 11:40 ` Will Deacon @ 2019-05-24 18:35 ` Will Deacon -1 siblings, 0 replies; 18+ messages in thread From: Will Deacon @ 2019-05-24 18:35 UTC (permalink / raw) To: Sami Tolvanen Cc: Mark Rutland, Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel, linux-arm-kernel Hi Sami, On Wed, May 15, 2019 at 12:40:39PM +0100, Will Deacon wrote: > On Tue, May 07, 2019 at 11:32:27AM -0700, Sami Tolvanen wrote: > > On Tue, May 07, 2019 at 06:25:12PM +0100, Mark Rutland wrote: > > > I strongly think that we cant to fix up the common definition in > > > kernel/sys_ni.c rather than having a point-hack in arm64. Other > > > architectures (e.g. x86, s390) will want the same for CFI, and I'd like > > > to ensure that our approached don't diverge. > > > > s390 already has the following in arch/s390/kernel/sys_s390.c: > > > > SYSCALL_DEFINE0(ni_syscall) > > { > > return -ENOSYS; > > } > > > > Which, I suppose, is cleaner than calling sys_ni_syscall. > > > > > I took a quick look, and it looks like it's messy but possible to fix > > > up the core. > > > > OK. How would you propose fixing this? > > In the absence of a patch from Mark, I'd suggest just adding a SYS_NI macro > to our asm/syscall_wrapper.h file which avoids the error injection stuff. It > doesn't preclude moving this to the core later on, but it unblocks the CFI > work. Do you plan to repost this? Will ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall @ 2019-05-24 18:35 ` Will Deacon 0 siblings, 0 replies; 18+ messages in thread From: Will Deacon @ 2019-05-24 18:35 UTC (permalink / raw) To: Sami Tolvanen Cc: Mark Rutland, Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel, linux-arm-kernel Hi Sami, On Wed, May 15, 2019 at 12:40:39PM +0100, Will Deacon wrote: > On Tue, May 07, 2019 at 11:32:27AM -0700, Sami Tolvanen wrote: > > On Tue, May 07, 2019 at 06:25:12PM +0100, Mark Rutland wrote: > > > I strongly think that we cant to fix up the common definition in > > > kernel/sys_ni.c rather than having a point-hack in arm64. Other > > > architectures (e.g. x86, s390) will want the same for CFI, and I'd like > > > to ensure that our approached don't diverge. > > > > s390 already has the following in arch/s390/kernel/sys_s390.c: > > > > SYSCALL_DEFINE0(ni_syscall) > > { > > return -ENOSYS; > > } > > > > Which, I suppose, is cleaner than calling sys_ni_syscall. > > > > > I took a quick look, and it looks like it's messy but possible to fix > > > up the core. > > > > OK. How would you propose fixing this? > > In the absence of a patch from Mark, I'd suggest just adding a SYS_NI macro > to our asm/syscall_wrapper.h file which avoids the error injection stuff. It > doesn't preclude moving this to the core later on, but it unblocks the CFI > work. Do you plan to repost this? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall 2019-05-24 18:35 ` Will Deacon @ 2019-05-24 21:58 ` Sami Tolvanen -1 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-24 21:58 UTC (permalink / raw) To: Will Deacon Cc: Mark Rutland, Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel, linux-arm-kernel On Fri, May 24, 2019 at 07:35:51PM +0100, Will Deacon wrote: > > In the absence of a patch from Mark, I'd suggest just adding a SYS_NI macro > > to our asm/syscall_wrapper.h file which avoids the error injection stuff. If we don't want to use SYSCALL_DEFINE0, I don't think we need a macro at all. I believe it's cleaner to just define __arm64_sys_ni_syscall with the correct type in sys.c. > Do you plan to repost this? Yes. Sorry for the delay. I'll post v3 shortly. Sami ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall @ 2019-05-24 21:58 ` Sami Tolvanen 0 siblings, 0 replies; 18+ messages in thread From: Sami Tolvanen @ 2019-05-24 21:58 UTC (permalink / raw) To: Will Deacon Cc: Mark Rutland, Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel, linux-arm-kernel On Fri, May 24, 2019 at 07:35:51PM +0100, Will Deacon wrote: > > In the absence of a patch from Mark, I'd suggest just adding a SYS_NI macro > > to our asm/syscall_wrapper.h file which avoids the error injection stuff. If we don't want to use SYSCALL_DEFINE0, I don't think we need a macro at all. I believe it's cleaner to just define __arm64_sys_ni_syscall with the correct type in sys.c. > Do you plan to repost this? Yes. Sorry for the delay. I'll post v3 shortly. Sami _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-05-24 21:58 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-03 19:12 [PATCH v2 0/3] fix function type mismatches in syscall wrappers Sami Tolvanen 2019-05-03 19:12 ` Sami Tolvanen 2019-05-03 19:12 ` [PATCH v2 1/3] arm64: fix syscall_fn_t type Sami Tolvanen 2019-05-03 19:12 ` Sami Tolvanen 2019-05-03 19:12 ` [PATCH v2 2/3] arm64: use the correct function type in SYSCALL_DEFINE0 Sami Tolvanen 2019-05-03 19:12 ` Sami Tolvanen 2019-05-03 19:12 ` [PATCH v2 3/3] arm64: use the correct function type for __arm64_sys_ni_syscall Sami Tolvanen 2019-05-03 19:12 ` Sami Tolvanen 2019-05-07 17:25 ` Mark Rutland 2019-05-07 17:25 ` Mark Rutland 2019-05-07 18:32 ` Sami Tolvanen 2019-05-07 18:32 ` Sami Tolvanen 2019-05-15 11:40 ` Will Deacon 2019-05-15 11:40 ` Will Deacon 2019-05-24 18:35 ` Will Deacon 2019-05-24 18:35 ` Will Deacon 2019-05-24 21:58 ` Sami Tolvanen 2019-05-24 21:58 ` Sami Tolvanen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.