Playing around with syscall framework macros broke the syscall tracing badly, see: commit bb3fbc44e783 ("arm64/syscalls: Allow syscalls to return capabilities") and: https://git.morello-project.org/morello/kernel/linux/-/issues/35 for the corresponding bug report.
This series in an attempt to remedy that. The first patch does some rather controversial thing, but there is already a precedent for that (as of ARCH_HAS_SYSCALL_MATCH_SYM_NAME), so one could say it can be somehow exonerated.
The second one tries to get the SYSCALL_METADATA macro aligned with the changes that caused the issue in the first place and, sadly, it is touching other archs code, though the changes are minimal and safe.
Note: checkpatch will complain about few things but those are to be ignored.
Tracing selftests seem to be happy with the changes. Also verified with some random syscall events.
Review branch: https://git.morello-project.org/Bea/linux/-/commits/morello/ftrace_syscalls/
--- BR B.
Beata Michalska (3): tracing/syscalls: Reshape arch_syscall_addr arm64:morello:tracing/syscalls: Get a grip on syscall tracing tracing/signal: Use explicit conversion instead of vague downcast
Documentation/trace/ftrace-design.rst | 5 +++-- arch/arm64/include/asm/ftrace.h | 4 ++++ arch/arm64/include/asm/syscall_wrapper.h | 9 ++++++--- arch/arm64/kernel/syscall.c | 12 ++++++++++++ arch/mips/include/asm/ftrace.h | 4 ++++ arch/s390/include/asm/syscall_wrapper.h | 6 +++--- arch/x86/include/asm/syscall_wrapper.h | 2 +- include/linux/syscalls.h | 24 +++++++++++++++--------- include/trace/events/signal.h | 4 ++-- kernel/trace/trace_syscalls.c | 4 +++- 10 files changed, 53 insertions(+), 21 deletions(-)
Despite having the generic arch_syscall_addr being weak, as per commit:
commit c763ba06bd9b ("tracing/syscalls: Make arch_syscall_addr weak")
the level of flexibility provided towards supporting customization of the sys_call_table is still insufficient for certain architectures, making arch_syscall_addr being susceptible to compile-time errors, whenever sys_call_table entries end up being more than just regular function pointer (as in the case of Morello and PCuABI :
commit bb3fbc44e783 ("arm64/syscalls: Allow syscalls to return capabilities")
Instead, drop the generic implementation (and assumptions made there) for archs that explicitly request not to use it.
Signed-off-by: Beata Michalska beata.michalska@arm.com --- Documentation/trace/ftrace-design.rst | 5 +++-- arch/mips/include/asm/ftrace.h | 4 ++++ kernel/trace/trace_syscalls.c | 4 +++- 3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/Documentation/trace/ftrace-design.rst b/Documentation/trace/ftrace-design.rst index 6893399157f0..d7c17901cb5a 100644 --- a/Documentation/trace/ftrace-design.rst +++ b/Documentation/trace/ftrace-design.rst @@ -241,8 +241,9 @@ You need very few things to get the syscalls tracing in an arch. - Put the trace_sys_enter() and trace_sys_exit() tracepoints calls from ptrace in the ptrace syscalls tracing path. - If the system call table on this arch is more complicated than a simple array - of addresses of the system calls, implement an arch_syscall_addr to return - the address of a given system call. + of addresses of the system calls or requires custom handling, define + ARCH_HAS_SYSCALL_ADDR in asm/ftrace.h and implement arch_syscall_addr to + return the address of a given system call. - If the symbol names of the system calls do not match the function names on this arch, define ARCH_HAS_SYSCALL_MATCH_SYM_NAME in asm/ftrace.h and implement arch_syscall_match_sym_name with the appropriate logic to return diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h index db497a8167da..a93b8e1c8d2c 100644 --- a/arch/mips/include/asm/ftrace.h +++ b/arch/mips/include/asm/ftrace.h @@ -10,6 +10,10 @@ #ifndef _ASM_MIPS_FTRACE_H #define _ASM_MIPS_FTRACE_H
+#ifdef CONFIG_FTRACE_SYSCALLS +#define ARCH_HAS_SYSCALL_ADDR +#endif + #ifdef CONFIG_FUNCTION_TRACER
#define MCOUNT_ADDR ((unsigned long)(_mcount)) diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c index f755bde42fd0..ad3bb8e8c651 100644 --- a/kernel/trace/trace_syscalls.c +++ b/kernel/trace/trace_syscalls.c @@ -516,10 +516,12 @@ struct trace_event_class __refdata event_class_syscall_exit = { .raw_init = init_syscall_trace, };
-unsigned long __init __weak arch_syscall_addr(int nr) +#ifndef ARCH_HAS_SYSCALL_ADDR +unsigned long __init arch_syscall_addr(int nr) { return (unsigned long)sys_call_table[nr]; } +#endif
void __init init_ftrace_syscalls(void) {
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 +#define ARCH_HAS_SYSCALL_ADDR +#endif + #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__) - +#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__) \ }; \ 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, ...)
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 + + #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__)
On 05/10/2022 16: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
broke "a" 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.
s/that those // ?
Does switching the args fix anything here or it's just to align with the order in __SYSCALL_DEFINEx?
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 +#define ARCH_HAS_SYSCALL_ADDR +#endif
#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__)
+#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__) \@@ -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, ...) 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
#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__)
On 06/10/2022 16:58, Zachary Leaf wrote:
On 05/10/2022 16: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
broke "a" 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.
s/that those // ?
Does switching the args fix anything here or it's just to align with the order in __SYSCALL_DEFINEx?
Nevermind - I've just seen your explanation of this on the bug report: https://git.morello-project.org/morello/kernel/linux/-/issues/35
Thanks, Zach
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 +#define ARCH_HAS_SYSCALL_ADDR +#endif
#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__)
+#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__) \@@ -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, ...) 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
#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__)
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
Hi Zachary, On Thu, Oct 06, 2022 at 04:58:10PM +0100, Zachary Leaf wrote:
On 05/10/2022 16: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
broke "a" number
Noted.
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.
s/that those // ?
That one was supposed to be 'does' ...
Does switching the args fix anything here or it's just to align with the order in __SYSCALL_DEFINEx?
This is actually the main fix - the SYSCALL_METADATA originally gets the arguments being swapped, which in othe case of arm64 is a problem as the syscall name is bing extended by the type of return value so the SYSCALL_DEFINE with (x, name ... ) will eventually get (x, name, ret_type, .....) but as SYSCALL_METADATA gets the order flipped, instead of getting (name, x ...) it will get (name, ret_type, x...) which causes all the problems. So what we are doing here is overriding SYSCALL_METADATA with original order of arguments, dropping the ret_type, and then flipping the order to feed that in into (what is now) __SYSCALL_METADATA.
--- BR B.
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 +#define ARCH_HAS_SYSCALL_ADDR +#endif
#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__)
+#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__) \@@ -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, ...) 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
#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__)
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
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.
+#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 :)
- #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.
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__)
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)
+#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)
--- 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__)
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__)
The trace event framework lacks support for capabilities (not on its own but does suffer from one) and casting a user pointer to a type that cannot actually hold a CHERI capability brings its own issues, so, for the time being, use user_ptr_addr to extract the address from a user pointer of interest.
Signed-off-by: Beata Michalska beata.michalska@arm.com --- include/trace/events/signal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h index 1db7e4b07c01..f3a83f8ce9ce 100644 --- a/include/trace/events/signal.h +++ b/include/trace/events/signal.h @@ -103,14 +103,14 @@ TRACE_EVENT(signal_deliver, __field( int, sig ) __field( int, errno ) __field( int, code ) - __field( unsigned long, sa_handler ) + __field( ptraddr_t, sa_handler ) __field( unsigned long, sa_flags ) ),
TP_fast_assign( __entry->sig = sig; TP_STORE_SIGINFO(__entry, info); - __entry->sa_handler = (unsigned long)ka->sa.sa_handler; + __entry->sa_handler = user_ptr_addr(ka->sa.sa_handler); __entry->sa_flags = ka->sa.sa_flags; ),
On 05/10/2022 16:08, Beata Michalska wrote:
The trace event framework lacks support for capabilities (not on its own but does suffer from one) [...]
It's not clear to me what is meant to be conveyed in the brackets here - maybe needs a rephrase?
[...] and casting a user pointer to a type that cannot actually hold a CHERI capability brings its own issues, so, for the time being, use user_ptr_addr to extract the address from a user pointer of interest.
Signed-off-by: Beata Michalska beata.michalska@arm.com
include/trace/events/signal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h index 1db7e4b07c01..f3a83f8ce9ce 100644 --- a/include/trace/events/signal.h +++ b/include/trace/events/signal.h @@ -103,14 +103,14 @@ TRACE_EVENT(signal_deliver, __field( int, sig ) __field( int, errno ) __field( int, code )
__field( unsigned long, sa_handler )
__field( ptraddr_t, sa_handler )TP_fast_assign( __entry->sig = sig; TP_STORE_SIGINFO(__entry, info);
__entry->sa_handler = (unsigned long)ka->sa.sa_handler;
__entry->sa_handler = user_ptr_addr(ka->sa.sa_handler);
On Thu, Oct 06, 2022 at 04:58:14PM +0100, Zachary Leaf wrote:
On 05/10/2022 16:08, Beata Michalska wrote:
The trace event framework lacks support for capabilities (not on its own but does suffer from one) [...]
It's not clear to me what is meant to be conveyed in the brackets here - maybe needs a rephrase?
It was supposed to clarify that the tracing framework does rely on printing capabilities, which on it's own, does not belong to that framework itself. Will try to rephrase.
Thanks.
--- BR B.
[...] and casting a user pointer to a type that cannot actually hold a CHERI capability brings its own issues, so, for the time being, use user_ptr_addr to extract the address from a user pointer of interest.
Signed-off-by: Beata Michalska beata.michalska@arm.com
include/trace/events/signal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h index 1db7e4b07c01..f3a83f8ce9ce 100644 --- a/include/trace/events/signal.h +++ b/include/trace/events/signal.h @@ -103,14 +103,14 @@ TRACE_EVENT(signal_deliver, __field( int, sig ) __field( int, errno ) __field( int, code )
__field( unsigned long, sa_handler )
__field( ptraddr_t, sa_handler )TP_fast_assign( __entry->sig = sig; TP_STORE_SIGINFO(__entry, info);
__entry->sa_handler = (unsigned long)ka->sa.sa_handler;
__entry->sa_handler = user_ptr_addr(ka->sa.sa_handler);
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 05/10/2022 16:08, Beata Michalska wrote:
Playing around with syscall framework macros broke the syscall tracing badly, see: commit bb3fbc44e783 ("arm64/syscalls: Allow syscalls to return capabilities") and: https://git.morello-project.org/morello/kernel/linux/-/issues/35 for the corresponding bug report.
This series in an attempt to remedy that. The first patch does some rather controversial thing, but there is already a precedent for that (as of ARCH_HAS_SYSCALL_MATCH_SYM_NAME), so one could say it can be somehow exonerated.
The second one tries to get the SYSCALL_METADATA macro aligned with the changes that caused the issue in the first place and, sadly, it is touching other archs code, though the changes are minimal and safe.
Note: checkpatch will complain about few things but those are to be ignored.
Tracing selftests seem to be happy with the changes. Also verified with some random syscall events.
Review branch: https://git.morello-project.org/Bea/linux/-/commits/morello/ftrace_syscalls/
BR B.
Beata Michalska (3): tracing/syscalls: Reshape arch_syscall_addr arm64:morello:tracing/syscalls: Get a grip on syscall tracing tracing/signal: Use explicit conversion instead of vague downcast
Tested-by: Zachary Leaf zachary.leaf@arm.com
Fixes build issues with CONFIG_FTRACE_SYSCALLS on Morello/arm64. Syscall tracing has been working on Morello board based on some limited testing.
Documentation/trace/ftrace-design.rst | 5 +++-- arch/arm64/include/asm/ftrace.h | 4 ++++ arch/arm64/include/asm/syscall_wrapper.h | 9 ++++++--- arch/arm64/kernel/syscall.c | 12 ++++++++++++ arch/mips/include/asm/ftrace.h | 4 ++++ arch/s390/include/asm/syscall_wrapper.h | 6 +++--- arch/x86/include/asm/syscall_wrapper.h | 2 +- include/linux/syscalls.h | 24 +++++++++++++++--------- include/trace/events/signal.h | 4 ++-- kernel/trace/trace_syscalls.c | 4 +++- 10 files changed, 53 insertions(+), 21 deletions(-)
On Thu, Oct 06, 2022 at 04:58:22PM +0100, Zachary Leaf wrote:
On 05/10/2022 16:08, Beata Michalska wrote:
Playing around with syscall framework macros broke the syscall tracing badly, see: commit bb3fbc44e783 ("arm64/syscalls: Allow syscalls to return capabilities") and: https://git.morello-project.org/morello/kernel/linux/-/issues/35 for the corresponding bug report.
This series in an attempt to remedy that. The first patch does some rather controversial thing, but there is already a precedent for that (as of ARCH_HAS_SYSCALL_MATCH_SYM_NAME), so one could say it can be somehow exonerated.
The second one tries to get the SYSCALL_METADATA macro aligned with the changes that caused the issue in the first place and, sadly, it is touching other archs code, though the changes are minimal and safe.
Note: checkpatch will complain about few things but those are to be ignored.
Tracing selftests seem to be happy with the changes. Also verified with some random syscall events.
Review branch: https://git.morello-project.org/Bea/linux/-/commits/morello/ftrace_syscalls/
BR B.
Beata Michalska (3): tracing/syscalls: Reshape arch_syscall_addr arm64:morello:tracing/syscalls: Get a grip on syscall tracing tracing/signal: Use explicit conversion instead of vague downcast
Tested-by: Zachary Leaf zachary.leaf@arm.com
Fixes build issues with CONFIG_FTRACE_SYSCALLS on Morello/arm64. Syscall tracing has been working on Morello board based on some limited testing.
Thanks for gining it a spin and for the review comments.
--- BR B
Documentation/trace/ftrace-design.rst | 5 +++-- arch/arm64/include/asm/ftrace.h | 4 ++++ arch/arm64/include/asm/syscall_wrapper.h | 9 ++++++--- arch/arm64/kernel/syscall.c | 12 ++++++++++++ arch/mips/include/asm/ftrace.h | 4 ++++ arch/s390/include/asm/syscall_wrapper.h | 6 +++--- arch/x86/include/asm/syscall_wrapper.h | 2 +- include/linux/syscalls.h | 24 +++++++++++++++--------- include/trace/events/signal.h | 4 ++-- kernel/trace/trace_syscalls.c | 4 +++- 10 files changed, 53 insertions(+), 21 deletions(-)
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 05/10/2022 17:08, Beata Michalska wrote:
Playing around with syscall framework macros broke the syscall tracing badly, see: commit bb3fbc44e783 ("arm64/syscalls: Allow syscalls to return capabilities") and: https://git.morello-project.org/morello/kernel/linux/-/issues/35 for the corresponding bug report.
This series in an attempt to remedy that. The first patch does some rather controversial thing, but there is already a precedent for that (as of ARCH_HAS_SYSCALL_MATCH_SYM_NAME), so one could say it can be somehow exonerated.
Looks perfectly reasonable to me, using __weak this way is not very robust.
The second one tries to get the SYSCALL_METADATA macro aligned with the changes that caused the issue in the first place and, sadly, it is touching other archs code, though the changes are minimal and safe.
I've got a suggestion to split that patch and a few other small things, otherwise I think that series is in good shape :)
Kevin
Note: checkpatch will complain about few things but those are to be ignored.
Tracing selftests seem to be happy with the changes. Also verified with some random syscall events.
Review branch: https://git.morello-project.org/Bea/linux/-/commits/morello/ftrace_syscalls/
BR B.
Beata Michalska (3): tracing/syscalls: Reshape arch_syscall_addr arm64:morello:tracing/syscalls: Get a grip on syscall tracing tracing/signal: Use explicit conversion instead of vague downcast
Documentation/trace/ftrace-design.rst | 5 +++-- arch/arm64/include/asm/ftrace.h | 4 ++++ arch/arm64/include/asm/syscall_wrapper.h | 9 ++++++--- arch/arm64/kernel/syscall.c | 12 ++++++++++++ arch/mips/include/asm/ftrace.h | 4 ++++ arch/s390/include/asm/syscall_wrapper.h | 6 +++--- arch/x86/include/asm/syscall_wrapper.h | 2 +- include/linux/syscalls.h | 24 +++++++++++++++--------- include/trace/events/signal.h | 4 ++-- kernel/trace/trace_syscalls.c | 4 +++- 10 files changed, 53 insertions(+), 21 deletions(-)
linux-morello@op-lists.linaro.org
-
Beata Michalska -
Kevin Brodsky -
Zachary Leaf