On 12/10/2022 12:10, Beata Michalska wrote:
On Tue, Oct 11, 2022 at 03:32:39PM +0200, Kevin Brodsky wrote:
On 05/10/2022 17:08, Beata Michalska wrote:
The alterations around the syscall framework made to enable support for capabilities, did spin the syscalls tracing out of control and broke number of things on the way. In order to get it back in line (eventually), make necessary adjustments to SYSCALL_METADATA and provide dedicated implementation for arch_syscall_addr.
SYSCALL_METADATA gets split into double-phased set-up, allowing archs to redefine how the arguments for the actual metadata handler macro are being arranged. On a side note, that those unify (at the surface at least) the order of the arguments used by syscall framework.
Fixes: bb3fbc44e783 ("arm64/syscalls: Allow syscalls to return capabilities") Signed-off-by: Beata Michalska beata.michalska@arm.com
arch/arm64/include/asm/ftrace.h | 4 ++++ arch/arm64/include/asm/syscall_wrapper.h | 9 ++++++--- arch/arm64/kernel/syscall.c | 12 ++++++++++++ arch/s390/include/asm/syscall_wrapper.h | 6 +++--- arch/x86/include/asm/syscall_wrapper.h | 2 +- include/linux/syscalls.h | 24 +++++++++++++++--------- 6 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h index 665373f441e1..0e277c532344 100644 --- a/arch/arm64/include/asm/ftrace.h +++ b/arch/arm64/include/asm/ftrace.h @@ -102,6 +102,10 @@ static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) return is_32bit_compat_task(); } +#ifdef CONFIG_CHERI_PURECAP_UABI
Should be #ifdef CONFIG_FTRACE_SYSCALLS too to be consistent.
True, though that would mean the arch_symbol_addr will have to be provided for arm64 even when PCuABI is not enabled, which would work but I's rather have it distinguished to have a clearer picture why it is needed. So I guess it would be: #if defined(CONFIG_FTRACE_SYSCALLS) && defined(CONFIG_CHERI_PURECAP_UABI)
Yes sorry that's what I meant, #ifdef on both :)
+#define ARCH_HAS_SYSCALL_ADDR +#endif
I would favour moving all the arm64-specific stuff to a separate patch, that would make it easier to see what's happening and also simplify your commit title :)
So I had that one split, but somehow having the chnages split into generic ones and arm64/Morello ones made me basically rewritting the same commit message, as the generic chenges are justified only by Morello changes. But I can see your point and I will split those.
- #define ARCH_HAS_SYSCALL_MATCH_SYM_NAME static inline bool arch_syscall_match_sym_name(const char *sym,
diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index b823aee59b6c..0ebee1ea0661 100644 --- a/arch/arm64/include/asm/syscall_wrapper.h +++ b/arch/arm64/include/asm/syscall_wrapper.h @@ -68,7 +68,7 @@ struct pt_regs; } #define __SYSCALL_DEFINE0(sname, ret_type) \
- SYSCALL_METADATA(sname, 0); \
- SYSCALL_METADATA(0, sname,); \ asmlinkage ret_type __arm64_sys##sname(const struct pt_regs *__unused); \ ALLOW_ERROR_INJECTION(__arm64_sys##sname, ERRNO); \ asmlinkage ret_type __arm64_compatentry_sys##sname(const struct pt_regs *__unused)\
@@ -94,7 +94,7 @@ struct pt_regs; #define __ARM64_SYS_STUBx(x, name, ...) #define __SYSCALL_DEFINE0(sname, ret_type) \
- SYSCALL_METADATA(sname, 0); \
- SYSCALL_METADATA(0, sname,); \ asmlinkage ret_type __arm64_sys##sname(const struct pt_regs *__unused); \ ALLOW_ERROR_INJECTION(__arm64_sys##sname, ERRNO); \ asmlinkage ret_type __arm64_sys##sname(const struct pt_regs *__unused)
@@ -141,7 +141,10 @@ struct pt_regs; #define __retptr__(name) name, _PTR #define __SYSCALL_ANNOTATE(name, ret_type) name, __SYSCALL_RET_T##ret_type #define SYSCALL_PREP(name, ...) __SYSCALL_ANNOTATE(_##name, __VA_ARGS__)
That one should stay.
+#ifdef CONFIG_FTRACE_SYSCALLS +#define SYSCALL_METADATA(x, name, ret_type, ...) \
- __SYSCALL_METADATA(name, x, __VA_ARGS__)
+#endif /* * Some syscalls with no parameters return valid capabilities, so __SYSCALL_DEFINE0 * is added to handle such cases. diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index a0e91ea7b74b..4942fcb4d299 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -105,6 +105,18 @@ static inline bool has_syscall_work(unsigned long flags) return unlikely(flags & _TIF_SYSCALL_WORK); } +#if defined(CONFIG_CHERI_PURECAP_UABI) && defined(CONFIG_FTRACE_SYSCALLS) +unsigned long __init arch_syscall_addr(int nr) +{
- /*
* In this particular case, it makes no difference,* which member of the syscall_entry_t instance is being* provided - the address is all that matters here*/- return (unsigned long)sys_call_table[nr].syscall_fn;
+} +#endif
- int syscall_trace_enter(struct pt_regs *regs); void syscall_trace_exit(struct pt_regs *regs);
diff --git a/arch/s390/include/asm/syscall_wrapper.h b/arch/s390/include/asm/syscall_wrapper.h index fde7e6b1df48..dee9c319734d 100644 --- a/arch/s390/include/asm/syscall_wrapper.h +++ b/arch/s390/include/asm/syscall_wrapper.h @@ -72,13 +72,13 @@ * named __s390x_sys_*() */ #define COMPAT_SYSCALL_DEFINE0(sname) \
- SYSCALL_METADATA(_##sname, 0); \
- SYSCALL_METADATA(0, _##sname); \ long __s390_compat_sys_##sname(void); \ ALLOW_ERROR_INJECTION(__s390_compat_sys_##sname, ERRNO); \ long __s390_compat_sys_##sname(void) #define SYSCALL_DEFINE0(sname) \
- SYSCALL_METADATA(_##sname, 0); \
- SYSCALL_METADATA(0, _##sname); \ long __s390x_sys_##sname(void); \ ALLOW_ERROR_INJECTION(__s390x_sys_##sname, ERRNO); \ long __s390_sys_##sname(void) \
@@ -129,7 +129,7 @@ #define __S390_SYS_STUBx(x, fullname, name, ...) #define SYSCALL_DEFINE0(sname) \
- SYSCALL_METADATA(_##sname, 0); \
- SYSCALL_METADATA(0, _##sname); \ long __s390x_sys_##sname(void); \ ALLOW_ERROR_INJECTION(__s390x_sys_##sname, ERRNO); \ long __s390x_sys_##sname(void)
diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h index 59358d1bf880..c39adc4810f0 100644 --- a/arch/x86/include/asm/syscall_wrapper.h +++ b/arch/x86/include/asm/syscall_wrapper.h @@ -247,7 +247,7 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs); * macros to work correctly. */ #define SYSCALL_DEFINE0(sname) \
- SYSCALL_METADATA(_##sname, 0); \
- SYSCALL_METADATA(0, _##sname); \ static long __do_sys_##sname(const struct pt_regs *__unused); \ __X64_SYS_STUB0(sname) \ __IA32_SYS_STUB0(sname) \
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index 8a6f561ce1de..c82cecc50675 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -193,12 +193,12 @@ extern struct trace_event_functions exit_syscall_print_funcs; __section("_ftrace_events") \ *__event_exit_##sname = &event_exit_##sname; -#define SYSCALL_METADATA(sname, nb, ...) \
- static const char *types_##sname[] = { \
__MAP(nb,__SC_STR_TDECL,__VA_ARGS__) \- }; \
- static const char *args_##sname[] = { \
__MAP(nb,__SC_STR_ADECL,__VA_ARGS__) \+#define __SYSCALL_METADATA(sname, nb, ...) \
- static const char *types_##sname[] = { \
__MAP(nb,__SC_STR_TDECL,__VA_ARGS__) \- }; \
- static const char *args_##sname[] = { \
__MAP(nb,__SC_STR_ADECL,__VA_ARGS__) \Only the first line has the intended change, the rest is tabs disappearing.
}; \ SYSCALL_TRACE_ENTER_EVENT(sname); \ SYSCALL_TRACE_EXIT_EVENT(sname); \@@ -224,7 +224,7 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event) } #else -#define SYSCALL_METADATA(sname, nb, ...) +#define __SYSCALL_METADATA(sname, nb, ...)
Might as well swap the arguments here too to be consistent.
You mean swapping the args for the actual metadata macro ? (so the above one)
Yes I mean having nb first for __SYSCALL_METADATA as well, so that we really have nb first everywhere.
Kevin
BR B.
static inline int is_syscall_trace_event(struct trace_event_call *tp_event) { @@ -232,6 +232,12 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event) } #endif +#ifndef SYSCALL_METADATA +#define SYSCALL_METADATA(nb, sname, ...) \
- __SYSCALL_METADATA(sname, nb, __VA_ARGS__)
+#endif
Only one empty line would do I think.
Kevin
#ifndef __retptr__ #define __retptr__(name) name #undef SYSCALL_PREP @@ -240,7 +246,7 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event) #ifndef SYSCALL_DEFINE0 #define SYSCALL_DEFINE0(sname) \
- SYSCALL_METADATA(_##sname, 0); \
- SYSCALL_METADATA(0, _##sname); \ asmlinkage long sys_##sname(void); \ ALLOW_ERROR_INJECTION(sys_##sname, ERRNO); \ asmlinkage long sys_##sname(void)
@@ -256,7 +262,7 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event) #define SYSCALL_DEFINE_MAXARGS 6 #define SYSCALL_DEFINEx(x, sname, ...) \
- SYSCALL_METADATA(sname, x, __VA_ARGS__) \
- SYSCALL_METADATA(x, sname, __VA_ARGS__) \ __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) #define __PROTECT(...) asmlinkage_protect(__VA_ARGS__)