ipc: userspace: don't fault when removing an already-sent IPC message - #11157
ipc: userspace: don't fault when removing an already-sent IPC message#11157kv2019i wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new verifier condition allows a userspace caller to bypass safety checks with a partially self-linked node (next==self, prev!=self), enabling unsafe list_item_del() writes through an attacker-controlled prev pointer.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adjusts the Zephyr userspace syscall verifier for ipc_msg_list_remove() to avoid a kernel oops when freeing an IPC message that has already been dequeued (i.e., its list node is self-linked/empty), which commonly occurs during stream stop / pipeline delete paths.
Changes:
- Relax
z_vrfy_ipc_msg_list_remove()verification to allow removing an already-dequeued (empty) message list node without faulting. - Add an explanatory comment documenting why empty/self-linked nodes are safe to remove and what scenario triggered the oops.
File summaries
| File | Description |
|---|---|
| src/ipc/ipc-common.c | Updates the userspace syscall verifier logic for IPC message list removal to avoid faulting on already-sent/dequeued messages. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * non-empty node that is not on ipc->msg_list, i.e. one whose list | ||
| * pointers would make list_item_del() corrupt unrelated memory. | ||
| */ | ||
| K_OOPS(K_SYSCALL_VERIFY(found || list_is_empty(&msg->list))); |
There was a problem hiding this comment.
Hmm, now that I think about it, Copilot actually has a point here, but I would not fix it the way Copilot suggests, but I would simply add:
if (list_is_empty(&msg->list))
return;
after the search loop, and have the original K_OOPS(K_SYSCALL_VERIFY(found)); after that.
There was a problem hiding this comment.
Thanks @jsarha , this is a good idea. Used this in the new version (just pushed).
PR 11157: test resultsRun date: 2026-09-07 14:04 UTC Tested commit: 01dbbb270519cd31207ac47ff497cbe854cd9940
|
jsarha
left a comment
jsarha
left a comment
There was a problem hiding this comment.
Good. I was a bit worried about the earlier check, but since I did not hit it in my tests, I assumed its Ok.
| * non-empty node that is not on ipc->msg_list, i.e. one whose list | ||
| * pointers would make list_item_del() corrupt unrelated memory. | ||
| */ | ||
| K_OOPS(K_SYSCALL_VERIFY(found || list_is_empty(&msg->list))); |
There was a problem hiding this comment.
Hmm, now that I think about it, Copilot actually has a point here, but I would not fix it the way Copilot suggests, but I would simply add:
if (list_is_empty(&msg->list))
return;
after the search loop, and have the original K_OOPS(K_SYSCALL_VERIFY(found)); after that.
86bd911 to
0d46446
Compare
z_vrfy_ipc_msg_list_remove() rejected any message that was not currently on ipc->msg_list by failing K_SYSCALL_VERIFY(found), which turns into a kernel oops. But ipc_msg_list_remove() is called from ipc_msg_free() / mod_ipc_msg_free() to drop a message that may or may not still be queued. The common case at stream stop / pipeline delete is freeing a message that has already been sent and dequeued: its list node is self-linked (empty), so it is not "found" and the verifier oopses the LL user thread with: <err> os.z_vrfy_ipc_msg_list_remove: syscall z_vrfy_ipc_msg_list_remove ... failed check: found <err> os.z_fatal_error: >>> ZEPHYR FATAL ERROR 3: Kernel oops on CPU 0 Relax the checks to avoid this scenario. If the msg->list is empty, we can return early. The msg->list pointer itself is already verified with K_SYSCALL_MEMORY_WRITE(). Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
0d46446 to
01dbbb2
Compare
|
V2&V3 pushed:
|
z_vrfy_ipc_msg_list_remove() rejected any message that was not currently on ipc->msg_list by failing K_SYSCALL_VERIFY(found), which turns into a kernel oops. But ipc_msg_list_remove() is called from ipc_msg_free() / mod_ipc_msg_free() to drop a message that may or may not still be queued. The common case at stream stop / pipeline delete is freeing a message that has already been sent and dequeued: its list node is self-linked (empty), so it is not "found" and the verifier oopses the LL user thread with:
os.z_vrfy_ipc_msg_list_remove: syscall z_vrfy_ipc_msg_list_remove
... failed check: found
os.z_fatal_error: >>> ZEPHYR FATAL ERROR 3: Kernel oops on CPU 0
Relax the checks to avoid this scenario. If the msg->list is empty, it is safe to call z_impl_ipc_msg_list_remove(). The msg->list pointer itself is already verified with K_SYSCALL_MEMORY_WRITE().