* [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.