Playing around with syscall framework macros broke the syscall tracing badly, see: commit ("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.
v3: - further splitting of patches ([PATCH 3/4] from v2) - aligning the order of #ifdef defines
v2: - split [PATCH 2/3] into generic and Morello specific changes - added dependency on CONFIG_FTRACE_SYSCALLS for ARCH_HAS_SYSCALL_ADDR - improved (hopefully) commit message for previous PATCH 2-3 - small formatting clean-up
Review branch: https://git.morello-project.org/Bea/linux/-/commits/morello/ftrace_syscalls_...
Beata Michalska (5): tracing/syscalls: Reshape arch_syscall_addr tracing/syscalls: Allow amending metadata macro arguments arm64/syscalls: Fix syscalls tracing arm64: morello: ftrace: Use dedicated arch_syscall_addr 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 | 8 ++++++-- 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 | 13 +++++++++---- include/trace/events/signal.h | 4 ++-- kernel/trace/trace_syscalls.c | 4 +++- 10 files changed, 47 insertions(+), 15 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 pointers (as in the case of Morello and PCuABI :
commit ("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 way in which the tracing framework defines its syscall metadata is tightly coupled with the syscall wrappers and those might be subject to architecture specific fine-tuning. For most cases this is not an issue, as SYSCALL_METADATA is simply piggy-backing on the arguments provided to those wrappers, though for arm64/Morello this proves to be problematic. To overcome this, split the SYSCALL_METADATA into a double-phased setup, allowing amending the arguments themselves, so that whatever changes the arch code decides to do there, get sanitised prior to expanding the actual metadata wrapper. Additionally, the order of the arguments here gets align with the rest of the syscall framework.
Signed-off-by: Beata Michalska beata.michalska@arm.com --- arch/arm64/include/asm/syscall_wrapper.h | 4 ++-- arch/s390/include/asm/syscall_wrapper.h | 6 +++--- arch/x86/include/asm/syscall_wrapper.h | 2 +- include/linux/syscalls.h | 13 +++++++++---- 4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index b823aee59b6c..97e08c4a6520 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) 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..4bd6d9a4c6ea 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -193,7 +193,7 @@ extern struct trace_event_functions exit_syscall_print_funcs; __section("_ftrace_events") \ *__event_exit_##sname = &event_exit_##sname;
-#define SYSCALL_METADATA(sname, nb, ...) \ +#define __SYSCALL_METADATA(nb, sname, ...) \ static const char *types_##sname[] = { \ __MAP(nb,__SC_STR_TDECL,__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(nb, sname, ...)
static inline int is_syscall_trace_event(struct trace_event_call *tp_event) { @@ -232,6 +232,11 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event) } #endif
+#ifndef SYSCALL_METADATA +#define SYSCALL_METADATA(nb, sname, ...) \ + __SYSCALL_METADATA(nb, sname, __VA_ARGS__) +#endif + #ifndef __retptr__ #define __retptr__(name) name #undef SYSCALL_PREP @@ -240,7 +245,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 +261,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__)
For the syscall framework to be able to recognize capabilities as a valid syscall return type, alterations were required, ending-up in modifying syscall wrappers and basically injecting an additional argument, that specifies the expected return type for given syscall handler. For some reason, SYSCALL_METADATA macro slipped under the radar, leaving it broken for arm64. The extra argument passed, crippled the overall logic for the tracing metadata macro, resulting in compile-time errors when CONFIG_FTRACE_SYSCALLS was enabled. This has been partially addressed by:
commit ("tracing/syscalls: Allow amending metadata macro arguments")
still, though, leaving SYSCALL_METADATA trying to process (unconsciously) the return type argument, which is bogus and leaves the tracing metadata in an overall mess. Make a full use of the above mentioned changes and drop the additional argument from the tracing metadata macro, restoring its original logic.
Fixes: ("arm64/syscalls: Allow syscalls to return capabilities") Signed-off-by: Beata Michalska beata.michalska@arm.com --- arch/arm64/include/asm/syscall_wrapper.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index 97e08c4a6520..30d4fd12e114 100644 --- a/arch/arm64/include/asm/syscall_wrapper.h +++ b/arch/arm64/include/asm/syscall_wrapper.h @@ -142,6 +142,10 @@ struct pt_regs; #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(x, name, __VA_ARGS__) +#endif /* * Some syscalls with no parameters return valid capabilities, so __SYSCALL_DEFINE0 * is added to handle such cases.
With the changes introduced to the syscall table for Morello & PCuABI:
commit ("arm64/syscalls: Allow syscalls to return capabilities")
the generic implementation for arch_syscall_addr is no longer valid, so provide a dedicated one, reinstating the syscall tracing when targeting PCuABI.
Signed-off-by: Beata Michalska beata.michalska@arm.com --- arch/arm64/include/asm/ftrace.h | 4 ++++ arch/arm64/kernel/syscall.c | 12 ++++++++++++ 2 files changed, 16 insertions(+)
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h index 665373f441e1..89ff574a4a91 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(); }
+#if defined(CONFIG_FTRACE_SYSCALLS) && defined(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/kernel/syscall.c b/arch/arm64/kernel/syscall.c index a0e91ea7b74b..6c45110431c8 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_FTRACE_SYSCALLS) && defined(CONFIG_CHERI_PURECAP_UABI) +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);
The trace event framework, at this point, lacks support for capabilities as there is no way to properly output those when logging events, 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 17/10/2022 16:30, Beata Michalska wrote:
For the syscall framework to be able to recognize capabilities as a valid syscall return type, alterations were required, ending-up in modifying syscall wrappers and basically injecting an additional argument, that specifies the expected return type for given syscall handler. For some reason, SYSCALL_METADATA macro slipped under the radar, leaving it broken for arm64. The extra argument passed, crippled the overall logic for the tracing metadata macro, resulting in compile-time errors when CONFIG_FTRACE_SYSCALLS was enabled. This has been partially addressed by:
commit ("tracing/syscalls: Allow amending metadata macro arguments")
still, though, leaving SYSCALL_METADATA trying to process (unconsciously) the return type argument, which is bogus and leaves the tracing metadata in an overall mess. Make a full use of the above mentioned changes and drop the additional argument from the tracing metadata macro, restoring its original logic.
Fixes: ("arm64/syscalls: Allow syscalls to return capabilities") Signed-off-by: Beata Michalska beata.michalska@arm.com
arch/arm64/include/asm/syscall_wrapper.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index 97e08c4a6520..30d4fd12e114 100644 --- a/arch/arm64/include/asm/syscall_wrapper.h +++ b/arch/arm64/include/asm/syscall_wrapper.h @@ -142,6 +142,10 @@ struct pt_regs; #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(x, name, __VA_ARGS__)
+#endif
Nit: it looks a bit strange without an empty line to separate it from the comment, I would add one.
Happy with the series otherwise, if you're happy with that I can add the newline when merging the series.
Kevin
/*
- Some syscalls with no parameters return valid capabilities, so __SYSCALL_DEFINE0
- is added to handle such cases.
On Tue, Oct 18, 2022 at 10:43:42AM +0200, Kevin Brodsky wrote:
On 17/10/2022 16:30, Beata Michalska wrote:
For the syscall framework to be able to recognize capabilities as a valid syscall return type, alterations were required, ending-up in modifying syscall wrappers and basically injecting an additional argument, that specifies the expected return type for given syscall handler. For some reason, SYSCALL_METADATA macro slipped under the radar, leaving it broken for arm64. The extra argument passed, crippled the overall logic for the tracing metadata macro, resulting in compile-time errors when CONFIG_FTRACE_SYSCALLS was enabled. This has been partially addressed by:
commit ("tracing/syscalls: Allow amending metadata macro arguments")
still, though, leaving SYSCALL_METADATA trying to process (unconsciously) the return type argument, which is bogus and leaves the tracing metadata in an overall mess. Make a full use of the above mentioned changes and drop the additional argument from the tracing metadata macro, restoring its original logic.
Fixes: ("arm64/syscalls: Allow syscalls to return capabilities") Signed-off-by: Beata Michalska beata.michalska@arm.com
arch/arm64/include/asm/syscall_wrapper.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index 97e08c4a6520..30d4fd12e114 100644 --- a/arch/arm64/include/asm/syscall_wrapper.h +++ b/arch/arm64/include/asm/syscall_wrapper.h @@ -142,6 +142,10 @@ struct pt_regs; #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(x, name, __VA_ARGS__)
+#endif
Nit: it looks a bit strange without an empty line to separate it from the comment, I would add one.
Happy with the series otherwise, if you're happy with that I can add the newline when merging the series.
If that's not too much to ask go ahead and amend the commit, otherwise let me know and I will respin the series with this fix.
On a side note: Zachary did add his 'tested-by' so he might want to have a second look at this version and decide whether to keep his tag on that one (?) @Zachary
Thanks.
--- B.
Kevin
/*
- Some syscalls with no parameters return valid capabilities, so __SYSCALL_DEFINE0
- is added to handle such cases.
On 18/10/2022 21:14, Beata Michalska wrote:
On Tue, Oct 18, 2022 at 10:43:42AM +0200, Kevin Brodsky wrote:
On 17/10/2022 16:30, Beata Michalska wrote:
For the syscall framework to be able to recognize capabilities as a valid syscall return type, alterations were required, ending-up in modifying syscall wrappers and basically injecting an additional argument, that specifies the expected return type for given syscall handler. For some reason, SYSCALL_METADATA macro slipped under the radar, leaving it broken for arm64. The extra argument passed, crippled the overall logic for the tracing metadata macro, resulting in compile-time errors when CONFIG_FTRACE_SYSCALLS was enabled. This has been partially addressed by:
commit ("tracing/syscalls: Allow amending metadata macro arguments")
still, though, leaving SYSCALL_METADATA trying to process (unconsciously) the return type argument, which is bogus and leaves the tracing metadata in an overall mess. Make a full use of the above mentioned changes and drop the additional argument from the tracing metadata macro, restoring its original logic.
Fixes: ("arm64/syscalls: Allow syscalls to return capabilities") Signed-off-by: Beata Michalska beata.michalska@arm.com
arch/arm64/include/asm/syscall_wrapper.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index 97e08c4a6520..30d4fd12e114 100644 --- a/arch/arm64/include/asm/syscall_wrapper.h +++ b/arch/arm64/include/asm/syscall_wrapper.h @@ -142,6 +142,10 @@ struct pt_regs; #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(x, name, __VA_ARGS__)
+#endif
Nit: it looks a bit strange without an empty line to separate it from the comment, I would add one.
Happy with the series otherwise, if you're happy with that I can add the newline when merging the series.
If that's not too much to ask go ahead and amend the commit, otherwise let me know and I will respin the series with this fix.
On a side note: Zachary did add his 'tested-by' so he might want to have a second look at this version and decide whether to keep his tag on that one (?) @Zachary
Sorry for the delay. I've built v3 and all seems to be working as expected still. Happy to retain the tag on the series. I haven't got any comments on the architectural side of things but I can at least attest that it works.
Thanks, Zach
Thanks.
B.
Kevin
/*
- Some syscalls with no parameters return valid capabilities, so __SYSCALL_DEFINE0
- is added to handle such cases.
On 19/10/2022 09:55, Zachary Leaf wrote:
On 18/10/2022 21:14, Beata Michalska wrote:
On Tue, Oct 18, 2022 at 10:43:42AM +0200, Kevin Brodsky wrote:
On 17/10/2022 16:30, Beata Michalska wrote:
For the syscall framework to be able to recognize capabilities as a valid syscall return type, alterations were required, ending-up in modifying syscall wrappers and basically injecting an additional argument, that specifies the expected return type for given syscall handler. For some reason, SYSCALL_METADATA macro slipped under the radar, leaving it broken for arm64. The extra argument passed, crippled the overall logic for the tracing metadata macro, resulting in compile-time errors when CONFIG_FTRACE_SYSCALLS was enabled. This has been partially addressed by:
commit ("tracing/syscalls: Allow amending metadata macro arguments")
still, though, leaving SYSCALL_METADATA trying to process (unconsciously) the return type argument, which is bogus and leaves the tracing metadata in an overall mess. Make a full use of the above mentioned changes and drop the additional argument from the tracing metadata macro, restoring its original logic.
Fixes: ("arm64/syscalls: Allow syscalls to return capabilities") Signed-off-by: Beata Michalska beata.michalska@arm.com
arch/arm64/include/asm/syscall_wrapper.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index 97e08c4a6520..30d4fd12e114 100644 --- a/arch/arm64/include/asm/syscall_wrapper.h +++ b/arch/arm64/include/asm/syscall_wrapper.h @@ -142,6 +142,10 @@ struct pt_regs; #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(x, name, __VA_ARGS__)
+#endif
Nit: it looks a bit strange without an empty line to separate it from the comment, I would add one.
Happy with the series otherwise, if you're happy with that I can add the newline when merging the series.
If that's not too much to ask go ahead and amend the commit, otherwise let me know and I will respin the series with this fix.
On a side note: Zachary did add his 'tested-by' so he might want to have a second look at this version and decide whether to keep his tag on that one (?) @Zachary
Sorry for the delay. I've built v3 and all seems to be working as expected still. Happy to retain the tag on the series. I haven't got any comments on the architectural side of things but I can at least attest that it works.
Cool thanks both. The series is now in next, with Zach's Tested-by and that newline added.
Kevin
Thanks, Zach
Thanks.
B.
Kevin
/* * Some syscalls with no parameters return valid capabilities, so __SYSCALL_DEFINE0 * is added to handle such cases.
linux-morello@op-lists.linaro.org
-
Beata Michalska -
Kevin Brodsky -
Zachary Leaf