Tilck on RISCV64 crashes in the system test `fs_perf1` when KMALLOC_FREE_MEM_POISONING=1
vvaltchev opened this issue · 6 comments
Hello @aenrbes,
another problem I've discovered with RISCV64 is that the kernel crashes when all the tests are run and KMALLOC_FREE_MEM_POISONING is enabled. It's very important to test the kernel with both UBSAN and KMALLOC_FREE_MEM_POISONING and make sure it works with all of those extra checks. It would be create to run tests also with an address sanitizer, but Tilck doesn't have support for that, yet.
Can you take a look? The backtrace is entirely in the generic code, but I've never observed that on i386. At the moment I believe the problem could be related with a subtle bug in the RISCV64 process - specific code.
Steps to reproduce:
- build Tilck for RISCV64
- run:
./build/st/run_all_tests -c -T sh
Fortunately, both this and the other issue (#227) can be reproduced locally 100% of the time, not just on Azure. Note: if you run only that test (fs_perf1) the problem cannot be reproduced. Therefore, something gets corrupted when all the tests run.
Vlad
[devshell] [RUN ] fs_perf1
Using '/tmp' as test dir
Avg. creat() cost: 118865 cycles
Avg. unlink() cost: 67110 cycles
********************** KERNEL PANIC **********************
kfree: Heap not found for block: 0xfaabcafefaabcafe
Current task [USER]: tid: 35, pid: 35
Interrupts: [ 8 ]
sstatus: 0x0000000000040022 [ SIE SPIE SUM ]
ra: 0x0000002000209e48, sp: 0x00000020380449d8, gp: 0x000000000004a420, tp: 0x000000000004e980
t0: 0x00000020000d7800, t1: 0x0000000000000000, t2: 0x0000002000204ae0, s0: 0x00000020380449f8
s1: 0x0000000000040020, a0: 0x0000000000000000, a1: 0x0000000000000000, a2: 0x0000000000000200
a3: 0x0000002000213bf6, a4: 0x0000002038044a80, a5: 0x0000002000290110, a6: 0x0000002007732024
a7: 0xffffffffffffff00, s2: 0x0000000000000000, s3: 0x0000000000000008, s4: 0x0000001ffffffbf0
s5: 0x0000000000000002, s6: 0x000000000000ff7f, s7: 0x00000191f886c35c, s8: 0x0000000000000000
s9: 0x0000000000000001, s10: 0x0000000000048e00, s11: 0x0000000000049870, t3: 0x0000000000000000
t4: 0x0000000000000010, t5: 0x0000000000000004, t6: 0x0000000000000006, sepc: 0x0000000000037fa4
sstatus: 0x0000000000040022, sbadaddr: 0x0000000000000000, scause: 0x0000000000000008
usersp: 0x0000000000000000
Stacktrace (12 frames):
[0x0000002000209fc4] panic + 0x308
[0x000000200022d4da] general_kfree + 0x17c
[0x00000020002345cc] kfree2 + 0x24
[0x0000002000235192] free_process_int + 0x16e
[0x00000020002352c6] free_task + 0x12a
[0x00000020002395e4] remove_task + 0xa6
[0x00000020002493dc] sys_wait4 + 0x3fe
[0x00000020002012d0] do_syscall_int + 0x4e
[0x0000002000201658] do_syscall + 0xec
[0x0000002000201796] handle_syscall + 0x82
[0x0000002000227cd4] syscall_entry + 0xc2
[0x000000200020b440] asm_trap_entry + 0xc0
System memory map from multiboot:
START END (T, Extr)
00) 0x0000000080000000 - 0x000000008001ffff (2, ) [ 128 KB]
01) 0x0000000080020000 - 0x00000000800fffff (1, ) [ 896 KB]
02) 0x0000000080100000 - 0x00000000801fffff (2, LMRS) [ 1024 KB]
03) 0x0000000080200000 - 0x000000008031afff (2, KRNL) [ 1132 KB]
04) 0x000000008031b000 - 0x00000000815fffff (1, ) [ 19348 KB]
05) 0x0000000081600000 - 0x000000008172bfff (2, RDSK) [ 1200 KB]
06) 0x000000008172c000 - 0x0000000087ffffff (1, ) [ 107344 KB]
Halting the system...
System halted.
In free_process_int() have the following code, the execution kfree_obj((void *)get_process_task(pi), struct task_and_process) will cause the value of pi->debug_cmdline to change to the poisoned value!!! Of course, pi should not be free before free pi->debug_cmdline.
if (release_obj(pi) == 0) {
arch_specific_free_proc(pi);
kfree_obj((void *)get_process_task(pi), struct task_and_process); ===========> Execution here will cause the value of `pi->debug_cmdline` to change to the poisoned value!
if (MOD_debugpanel)
kfree2(pi->debug_cmdline, PROCESS_CMDLINE_BUF_SIZE);
}
So the solution is quite simple, Just change it as following, (and I won't submit the PR myself)
if (release_obj(pi) == 0) {
if (MOD_debugpanel)
kfree2(pi->debug_cmdline, PROCESS_CMDLINE_BUF_SIZE);
arch_specific_free_proc(pi);
kfree_obj((void *)get_process_task(pi), struct task_and_process);
}
Thank you @aenrbes. I completely agree that's a bug. I'll try the fix you suggested later today after my work time.
However, I fail to understand how we don't hit that on i386 too all the time: all the CI builds use the free kmalloc poisoning and MOD_debugpanel is compiled in. We should be hitting this 100% of time because free_process_int() is generic code, while we never did. Something must be different on RISCV64, right? Do you have any thoughts?
@vvaltchev
Since KMALLOC_FREE_MEM_POISONING will only work in main_heaps_kfree(), free_process_int() will only execute to small_heaps_kfree() in most cases, main_heaps_kfree() is not executed at all. In other words, in most cases, KMALLOC_FREE_MEM_POISONING didn't work at all; I guess because riscv64 is 64-bit, main_heaps_kfree() will be executed in some cases.
Thank you @aenrbes ! That makes absolutely sense. Two thoughts, FYI:
-
I should make KMALLOC_FREE_MEM_POISONING work also for the small heaps. I don't know why it works only for the "big" heap allocations.
-
I didn't realize that the process + task struct is so big on RISCV64 that doesn't fit in 2 KB anymore. I'll check their sizes and see what's the best approach. Either make two separate allocations, or make the small heap size to be 4 KB instead.
The size of riscv64 's process + task struct should be less than 2kb, because main_heaps_free() is only triggered on rare occasions, so the problem should be that the 64bit platform runs out of memory in small heaps on rare occasions
Thanks for the hint! Makes sense to me.
I'll research that when I have a bit of extra time. If we run out of small heaps, we should have more small heaps. Certainly with 64-bit the memory usage is different due to the larger data structures.