vm/map: Message buffers memory safety#810
Conversation
TASK: RTOS-1393
There was a problem hiding this comment.
Code Review
This pull request introduces vm_mapBorrow to allow borrowing virtual memory map entries from a source map to a destination map, and updates proc/msg.c to use this function for bulk page mapping instead of mapping pages individually. Feedback on these changes highlights several critical issues in vm_mapBorrow, including an incorrect loop condition comparing a cumulative offset to a chunk size, a stale pointer risk due to an unreset variable e across iterations, and a security vulnerability where copy-on-write protections (MAP_NEEDSCOPY) are bypassed. Additionally, reviewers noted an uninitialized err variable in proc/msg.c and recommended refactoring error handling in vm_mapBorrow to adhere to MISRA 15.5 single-exit-point guidelines.
Unit Test Results10 813 tests - 77 10 116 ✅ - 86 1h 5m 7s ⏱️ + 6m 18s For more details on these failures, see this check. Results for commit 15b8bd6. ± Comparison against base commit fb914a8. This pull request removes 82 and adds 5 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Update message receiver map entries with corresponding amap/objects to ensure these pages won't get allocated by keeping reference counters non-zero even when message sender unmaps these sent pages. TASK: RTOS-1393
b6fccf3 to
15b8bd6
Compare
Description
Update message receiver map entries with corresponding amap/objects to ensure these pages won't get allocated by keeping reference counters non-zero even when message sender unmaps these sent pages.
Example that causes kernel panics on ia32 via the problem described above:
These changes exacerbate that issue: phoenix-rtos/phoenix-rtos-project#1714, , awaiting for fix
Motivation and Context
Current msg implementation maps pages without marking them anywhere, leading to potential reallocation and multimapping such pages in other places (even as kernel structures).
Types of changes
How Has This Been Tested?
Checklist:
Special treatment