unexpected behavior when doing syscall with 7 or more arguments #29386
Assignees
Labels
Comments
|
My general experience is that every time I've suspected a compiler bug, I've been wrong, except once. |
@andrewboie sure, z_arm_do_syscall is in the path.
maybe I'm wrong, but compiler puts the 7th argument on the wrong place... |
diff --git a/tests/kernel/mem_protect/syscalls/src/main.c b/tests/kernel/mem_protect/syscalls/src/main.c
index ddf6a38463..b86276d519 100644
--- a/tests/kernel/mem_protect/syscalls/src/main.c
+++ b/tests/kernel/mem_protect/syscalls/src/main.c
@@ -201,6 +201,37 @@ void test_to_copy(void)
K_MEM_POOL_DEFINE(test_pool, BUF_SIZE, BUF_SIZE, 4, 4);
+uint32_t z_impl_sum_arg7(uint32_t arg1, uint32_t arg2, uint32_t arg3,
+ uint32_t arg4, uint32_t arg5, uint32_t arg6,
+ uint32_t arg7)
+{
+ uint32_t args[] = { arg1, arg2, arg3, arg4, arg5, arg6, arg7 };
+ uint32_t ret = 0xaef464;
+
+ for (int i = 0; i < ARRAY_SIZE(args); i++) {
+ ret += args[i];
+ ret = (ret << 11) | (ret >> 23);
+ }
+
+ return ret;
+}
+
+Z_SYSCALL_HANDLER(sum_arg7, arg1, arg2, arg3, arg4, arg5, more_args_ptr)
+{
+ volatile struct _syscall_7_args *margs =
+ (struct _syscall_7_args *)more_args_ptr;
+
+ return z_impl_sum_arg7(arg1, arg2, arg3, arg4, arg5,
+ margs->arg6, margs->arg7);
+}
+
+void test_arg7(void)
+{
+ uint32_t ret = sum_arg7(1, 2, 3, 4, 5, 6, 7);
+
+ printk("the sum is %d\n", ret);
+}
+
void test_main(void)
{
sprintf(kernel_string, "this is a kernel string");
@@ -211,6 +242,7 @@ void test_main(void)
ztest_unit_test(test_string_nlen),
ztest_user_unit_test(test_string_nlen),
ztest_user_unit_test(test_to_copy),
+ ztest_user_unit_test(test_arg7),
ztest_user_unit_test(test_user_string_copy),
ztest_user_unit_test(test_user_string_alloc_copy));
ztest_run_test_suite(syscalls);
diff --git a/tests/kernel/mem_protect/syscalls/src/test_syscalls.h b/tests/kernel/mem_protect/syscalls/src/test_syscalls.h
index 9c0645b008..81991406ad 100644
--- a/tests/kernel/mem_protect/syscalls/src/test_syscalls.h
+++ b/tests/kernel/mem_protect/syscalls/src/test_syscalls.h
@@ -16,6 +16,10 @@ __syscall int to_copy(char *dest);
__syscall size_t string_nlen(char *src, size_t maxlen, int *err);
+__syscall uint32_t sum_arg7(uint32_t arg1, uint32_t arg2, uint32_t arg3,
+ uint32_t arg4, uint32_t arg5, uint32_t arg6,
+ uint32_t arg7);
+
#include <syscalls/test_syscalls.h>
#endif /* _TEST_SYSCALLS_H_ */above code on 1.14 branch with 0.10.3 SDK will also cause mpu fault. checking... |
|
same error on mps2_an385 both with SDK 0.10.3 and SDK 0.11.4 but qemu_x86 works well with SDK 0.10.3 |
diff --git a/arch/arm/core/userspace.S b/arch/arm/core/userspace.S
index 60888c08cb..6c492cae9d 100644
--- a/arch/arm/core/userspace.S
+++ b/arch/arm/core/userspace.S
@@ -233,7 +233,8 @@ SECTION_FUNC(TEXT, z_arm_do_syscall)
valid_syscall:
/* push args to complete stack frame */
- push {r4,r5}
+ mov ip, sp
+ push {r4,r5,ip}
dispatch_syscall:
ldr ip, =_k_syscall_table
@@ -244,7 +245,7 @@ dispatch_syscall:
blx ip
/* restore LR */
- ldr lr, [sp,#12]
+ ldr lr, [sp,#16]
#if defined(CONFIG_BUILTIN_STACK_GUARD)
/* clear stack limit (stack protection not required in user mode) */
@@ -253,7 +254,7 @@ dispatch_syscall:
#endif
/* set stack back to unprivileged stack */
- ldr ip, [sp,#8]
+ ldr ip, [sp,#12]
msr PSP, ip
push {r0, r1}the root cause is that we don't push ssf to the stack, and almost all of the handlers don't use it, so compiler replace the memory of ssf as above assembly code behaves. |
|
Nice work, thanks @wentongwu |
@andrewboie sure, I will. |
wentongwu
added a commit
to wentongwu/zephyr
that referenced
this issue
Oct 26, 2020
All system call handlers have the same prototype: u32_t _handler_APINAME(u32_t arg1, u32_t arg2, u32_t arg3, u32_t arg4, u32_t arg5, u32_t arg6, void *ssf); This commit pushes the seventh argument named ssf to thread's privileged stack to avoid mis-understanding the stack layout with compiler. Fixes: zephyrproject-rtos#29386. Signed-off-by: Wentong Wu <[email protected]>
@andrewboie please review PR for 1.14 branch first, I'm considering how to create test case and PR for master. |
wentongwu
added a commit
to wentongwu/zephyr
that referenced
this issue
Oct 28, 2020
Pushes the seventh argument named ssf to thread's privileged stack to follow below syscall prototype. uintptr_t z_mrsh_xx(uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3, uintptr_t arg4, void *more, void *ssf) Fixes: zephyrproject-rtos#29386. Signed-off-by: Wentong Wu <[email protected]>
nashif
pushed a commit
that referenced
this issue
Nov 12, 2020
Pushes the seventh argument named ssf to thread's privileged stack to follow below syscall prototype. uintptr_t z_mrsh_xx(uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3, uintptr_t arg4, void *more, void *ssf) Fixes: #29386. Signed-off-by: Wentong Wu <[email protected]>
nashif
pushed a commit
that referenced
this issue
Nov 16, 2020
All system call handlers have the same prototype: u32_t _handler_APINAME(u32_t arg1, u32_t arg2, u32_t arg3, u32_t arg4, u32_t arg5, u32_t arg6, void *ssf); This commit pushes the seventh argument named ssf to thread's privileged stack to avoid mis-understanding the stack layout with compiler. Fixes: #29386. Signed-off-by: Wentong Wu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Got some interesting code as below which is running on Cortex-M7 with user space enabled, the tool-chain is Zephyr SDK 0.10.3(somehow it's not easy to update it to latest one) and the code base is 1.14 branch.
The code from
60005bfcto60005c1eis doing arguments preparation to call api->chnl_read_async((uint32_t)&(((struct adc_driver_api *)0)->chnl_read_async) = 36), but the code located at60005c08(strd r2, r1, [sp, #52]) which is trying to putmargs->arg6at[sp, #52]and putmargs->arg7at[sp, #56]will cause the stack (before calling this function) overflow 4 bytes(it may be easily understood with below layout of the privileged stack of the calling thread). So the unprivileged SP will be wrong when back to original function that called SVC.And I made a patch temporally(if more arguments, need push more. It's not solution) as below to workaround it and it will work as expected. But we need find out why compiler behaves like this, currently IMHO it's most probably a bug of compiler, welcome any idea. Maybe we also need add some test cases to cover more than 6 arguments syscall.
The text was updated successfully, but these errors were encountered: