search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.64k stars 6.51k forks source link

tests: kernel: thread_apis: test may fail on targets where assumed invalid pointer address is valid memory #79627

Open ibirnbaum opened 11 hours ago

ibirnbaum commented 11 hours ago

I have begun an effort to re-work the pretty ancient MPU regions configuration for the Xilinx ZynqMP, which is simulated in qemu_cortex_r5. Probably the biggest flaw of what we've had for the past few years now is the fact that the MPU regions weren't even associated with the device tree in any way.

As a minor benefit of the whole overhaul, which has been PR'ed as #79221, a new memory mapping has been added which causes a problem with the thread_apis testsuite, namely the test_thread_name_user_get_set test case.

Until now, the last 128MB below the 4G boundary are mapped as device memory with permissions P:RW, U:NA. This large block of memory contains the ZynqMP's peripherals' register spaces. Due to the limited number of MPU region entries, a more fine-grained approach at mapping peripherals on the basis of what is actually enabled (comp. the Zynq-7000 which has a MMU, most peripherals are 4k register spaces) isn't possible.

With this configuration, the test case test_thread_name_user_get_set, which tries to force an invalid memory access by using a hardcoded str pointer of 0xFFFFFFF0 while in userspace, passes as expected as the memory access actually fails. This is, however, thanks to the particular memory layout and according configuration of the ZynqMP's MPU, while I guess that it was assumed for the testcase that 0xFFFFFFF0 might be an invalid address in general.

With #79221, I've added one more memory region overlapping with the peripherals mapping. Is the full 128M below 4G used by the peripherals? No, the last 256k below 4G are On-Chip Memory, which have been configured as user-accessible.

With this updated configuration, the test_thread_name_user_get_set test case fails as the assumed bogus address has become valid, usable memory.

Regardless of this particular example, it probably shouldn't be assumed that one particular memory address is universally invalid/inaccessible.

Code base: https://github.com/ibirnbaum/zephyr.git, branch zyqnmp_rpu_mpu (yes, that's an unfortunate typo there), SDK 0.16.8, Ubuntu 22. Build command: west build -p -b qemu_cortex_r5 tests/kernel/threads/thread_apis -T kernel.threads.apis

Output in case of the test case failing (comp. output of twister-build(10) in #79221):

START - test_thread_name_user_get_set
Assertion failed at WEST_TOPDIR/zephyr/tests/kernel/threads/thread_apis/src/main.c:161: threads_lifecycle_test_thread_name_user_get_set: (ret not equal to -EFAULT)
accepted nonsense string (0)
FAIL - test_thread_name_user_get_set in 0.008 seconds

Changing the address in line 160 to an actually inaccessible address will make the test case pass.

Expected: test case passing when attempting to access an address that is actually unavailable, plus the test case not using an address that is valid memory on the current target.

This issue is preventing the twister tests run for #79221 from passing.

ibirnbaum commented 11 hours ago

Would it be possible to have some sort of override parameter for tests where invalid memory is accessed deliberately, or would that be exaggerated in case that only this one test case is affected by the described effect?