phoenix-rtos / phoenix-rtos-project

Sample project using Phoenix-RTOS
https://phoenix-rtos.com
BSD 3-Clause "New" or "Revised" License
43 stars 32 forks source link

vm: `_page_get()` may cause unexpected behavior #1074

Open niewim19 opened 4 months ago

niewim19 commented 4 months ago

Function that can cause a problem: phoenix-rtos-kernel/vm/page.c:_page_get(): https://github.com/phoenix-rtos/phoenix-rtos-kernel/blob/f29031f5ca6707f70e2720819f1255828a0c5cba/vm/page.c#L177

The problem: _page_get() searches through the binary tree without locking the pages lock. This function is used here: https://github.com/phoenix-rtos/phoenix-rtos-kernel/blob/f29031f5ca6707f70e2720819f1255828a0c5cba/vm/map.c#L949

There is possibility that one thread indirectly executes page deallocation (for example through msg_respond()->msg_release()->vm_pageFree()) while other thread indirectly seeks pages (syscalls_beginthreadex()->proc_put()->process_destroy()->vm_mapDestroy()-> _page_get()) then the binary tree might be changed while it is searched which leads to undefined behavior. I did not examine in detail this approach (maybe other lock prevents it) but there is also other use in https://github.com/phoenix-rtos/phoenix-rtos-kernel/blob/f29031f5ca6707f70e2720819f1255828a0c5cba/vm/object.c#L201 which is also unguarded.

astalke commented 3 months ago

I've made a test branch that adds guarding locks and fortunately it doesn't cause a deadlock. I haven't checked yet if there is another lock protecting everything.

astalke commented 3 months ago

There is no lock in the call chain: