* [PATCH 0/2] fix function type mismatches in syscall wrappers
@ 2019-05-01 20:04 Sami Tolvanen
2019-05-01 20:04 ` [PATCH 1/2] arm64: fix syscall_fn_t type Sami Tolvanen
2019-05-01 20:04 ` [PATCH 2/2] arm64: use the correct function type in SYSCALL_DEFINE0 Sami Tolvanen
0 siblings, 2 replies; 6+ messages in thread
From: Sami Tolvanen @ 2019-05-01 20:04 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
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.
Sami Tolvanen (2):
arm64: fix syscall_fn_t type
arm64: use the correct function type in SYSCALL_DEFINE0
arch/arm64/include/asm/syscall.h | 2 +-
arch/arm64/include/asm/syscall_wrapper.h | 18 +++++++++---------
2 files changed, 10 insertions(+), 10 deletions(-)
--
2.21.0.593.g511ec345e18-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] 6+ messages in thread
* [PATCH 1/2] arm64: fix syscall_fn_t type
2019-05-01 20:04 [PATCH 0/2] fix function type mismatches in syscall wrappers Sami Tolvanen
@ 2019-05-01 20:04 ` Sami Tolvanen
2019-05-03 10:11 ` Mark Rutland
2019-05-01 20:04 ` [PATCH 2/2] arm64: use the correct function type in SYSCALL_DEFINE0 Sami Tolvanen
1 sibling, 1 reply; 6+ messages in thread
From: Sami Tolvanen @ 2019-05-01 20:04 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Sami Tolvanen, Nick Desaulniers, Kees Cook, linux-arm-kernel,
linux-kernel
Use const struct pt_regs * instead of struct pt_regs * as
the argument type to fix indirect call type mismatches with
Control-Flow Integrity checking.
Signed-off-by: Sami Tolvanen <samitolvanen@google.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.593.g511ec345e18-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] 6+ messages in thread
* [PATCH 2/2] arm64: use the correct function type in SYSCALL_DEFINE0
2019-05-01 20:04 [PATCH 0/2] fix function type mismatches in syscall wrappers Sami Tolvanen
2019-05-01 20:04 ` [PATCH 1/2] arm64: fix syscall_fn_t type Sami Tolvanen
@ 2019-05-01 20:04 ` Sami Tolvanen
2019-05-03 10:21 ` Mark Rutland
1 sibling, 1 reply; 6+ messages in thread
From: Sami Tolvanen @ 2019-05-01 20:04 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
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.593.g511ec345e18-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] 6+ messages in thread
* Re: [PATCH 1/2] arm64: fix syscall_fn_t type
2019-05-01 20:04 ` [PATCH 1/2] arm64: fix syscall_fn_t type Sami Tolvanen
@ 2019-05-03 10:11 ` Mark Rutland
0 siblings, 0 replies; 6+ messages in thread
From: Mark Rutland @ 2019-05-03 10:11 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel,
Will Deacon, linux-arm-kernel
On Wed, May 01, 2019 at 01:04:50PM -0700, Sami Tolvanen wrote:
> Use const struct pt_regs * instead of struct pt_regs * as
> the argument type to fix indirect call type mismatches with
> Control-Flow Integrity checking.
It's probably worth noting that in <asm/syscall_wrapper.h> all syscall
wrappers take a const struct pt_regs *, which is where the mismatch
comes from.
>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.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);
For a second I was worried that we modify the regs to assign the return
value, but I see we do that in the syscall.c wrapper, where the pt_regs
argument isn't const.
We certainly chouldn't need to modify the regs when acquiring the
arguments, and as above this matches <asm/syscall_wrapper.h>, so this
looks sound to me.
FWIW:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
_______________________________________________
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] 6+ messages in thread
* Re: [PATCH 2/2] arm64: use the correct function type in SYSCALL_DEFINE0
2019-05-01 20:04 ` [PATCH 2/2] arm64: use the correct function type in SYSCALL_DEFINE0 Sami Tolvanen
@ 2019-05-03 10:21 ` Mark Rutland
2019-05-03 17:02 ` Sami Tolvanen
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2019-05-03 10:21 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel,
Will Deacon, linux-arm-kernel
On Wed, May 01, 2019 at 01:04:51PM -0700, Sami Tolvanen wrote:
> 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.
Generally, this makes sense, but I'm not sure that this is complete.
IIUC this introduces a new type mismatch with sys_ni_syscall() in some
cases. We probably need that to use SYSCALL_DEFINE0(), and maybe have a
ksys_ni_syscall() for in-kernel wrappers.
Thanks,
Mark.
>
> 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.593.g511ec345e18-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] 6+ messages in thread
* Re: [PATCH 2/2] arm64: use the correct function type in SYSCALL_DEFINE0
2019-05-03 10:21 ` Mark Rutland
@ 2019-05-03 17:02 ` Sami Tolvanen
0 siblings, 0 replies; 6+ messages in thread
From: Sami Tolvanen @ 2019-05-03 17:02 UTC (permalink / raw)
To: Mark Rutland
Cc: Kees Cook, Catalin Marinas, Nick Desaulniers, linux-kernel,
Will Deacon, linux-arm-kernel
Hi Mark,
On Fri, May 03, 2019 at 11:21:28AM +0100, Mark Rutland wrote:
> Generally, this makes sense, but I'm not sure that this is complete.
>
> IIUC this introduces a new type mismatch with sys_ni_syscall() in some
> cases.
Thanks for the review. You're correct, sys_ni_syscall needs to be fixed
too. I'll include this in v2.
> We probably need that to use SYSCALL_DEFINE0(), and maybe have a
> ksys_ni_syscall() for in-kernel wrappers.
Why would we need ksys_ni_syscall? It seems something like this should
be sufficient:
asmlinkage long sys_ni_syscall(void);
SYSCALL_DEFINE0(ni_syscall)
{
return sys_ni_syscall();
}
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] 6+ messages in thread
end of thread, other threads:[~2019-05-03 17:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-01 20:04 [PATCH 0/2] fix function type mismatches in syscall wrappers Sami Tolvanen
2019-05-01 20:04 ` [PATCH 1/2] arm64: fix syscall_fn_t type Sami Tolvanen
2019-05-03 10:11 ` Mark Rutland
2019-05-01 20:04 ` [PATCH 2/2] arm64: use the correct function type in SYSCALL_DEFINE0 Sami Tolvanen
2019-05-03 10:21 ` Mark Rutland
2019-05-03 17:02 ` Sami Tolvanen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).