search

vvaltchev / tilck

A Tiny Linux-Compatible Kernel
BSD 2-Clause "Simplified" License
2.35k stars 100 forks source link

Tilck on RISCV64 crashes in the system test `fs_perf1` when KMALLOC_FREE_MEM_POISONING=1 #228

Closed vvaltchev closed 3 days ago

vvaltchev commented 5 days ago

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:

Fortunately, both this and the other issue (https://github.com/vvaltchev/tilck/issues/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.
aenrbes commented 3 days ago

@vvaltchev

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);
   }
vvaltchev commented 3 days ago

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?

aenrbes commented 3 days ago

@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.

vvaltchev commented 3 days ago

Thank you @aenrbes ! That makes absolutely sense. Two thoughts, FYI:

aenrbes commented 3 days ago

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

vvaltchev commented 3 days ago

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.