zephyr: vregion: stop logging spurious "read access denied" on free p… - #11163
zephyr: vregion: stop logging spurious "read access denied" on free p…#11163kv2019i wants to merge 1 commit into
Conversation
…aths vregion_verify() asserts that the vregion metadata object is NOT accessible to the userspace context. It did this with K_OOPS(!K_SYSCALL_MEMORY_READ(vr, sizeof(*vr))); but K_SYSCALL_MEMORY_READ() emits an "os.vregion_verify: ... Memory region <addr> (size 88) read access denied" error via LOG_ERR precisely when the region is inaccessible - i.e. in the expected, correct case for a kernel-only vregion. Probe the mapping directly with arch_buffer_validate(), which performs the same check without logging, and oops only if the userspace context can actually read the metadata. No functional change to the security check; only the false-positive error logging is removed. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to suppress expected-denial logging while retaining the same access validation behavior.
Pull request overview
Removes false-positive “read access denied” logging in vregion_verify() by replacing K_SYSCALL_MEMORY_READ() with a direct arch_buffer_validate() probe, preserving the security check semantics while avoiding expected-denial error logs in userspace syscall verification paths.
Changes:
- Replace
K_SYSCALL_MEMORY_READ()witharch_buffer_validate()invregion_verify()to avoid spurious LOG_ERR output. - Expand inline comments to document why the syscall helper is not appropriate here.
File summaries
| File | Description |
|---|---|
zephyr/lib/vregion.c |
Switches vregion metadata accessibility verification to a non-logging probe to prevent expected “access denied” errors from being logged. |
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.
| * expected (good) case for a kernel-only vregion. Omit false | ||
| * error messages by using arch_buffer_validate() directly. | ||
| */ | ||
| K_OOPS(arch_buffer_validate((void *)vr, sizeof(*vr), 0) == 0); |
There was a problem hiding this comment.
to be pedantic we should #include <zephyr/arch/arch_interface.h> but it's guaranteed that it's included via zephyr/internal/syscall_handler.h, so, I'd say it isn't a requirement. Compilation won't fail.
| * expected (good) case for a kernel-only vregion. Omit false | ||
| * error messages by using arch_buffer_validate() directly. | ||
| */ | ||
| K_OOPS(arch_buffer_validate((void *)vr, sizeof(*vr), 0) == 0); |
There was a problem hiding this comment.
to be pedantic we should #include <zephyr/arch/arch_interface.h> but it's guaranteed that it's included via zephyr/internal/syscall_handler.h, so, I'd say it isn't a requirement. Compilation won't fail.
PR 11163: test resultsRun date: 2026-09-03 13:54 UTC Tested commit: 360be17109e0cd5ab6d1cb2cf08e75f2038ec6dd
|
…aths
vregion_verify() asserts that the vregion metadata object is NOT accessible to the userspace context. It did this with
but K_SYSCALL_MEMORY_READ() emits an "os.vregion_verify: ... Memory region (size 88) read access denied" error via LOG_ERR precisely when the region is inaccessible - i.e. in the expected, correct case for a kernel-only vregion.
Probe the mapping directly with arch_buffer_validate(), which performs the same check without logging, and oops only if the userspace context can actually read the metadata. No functional change to the security check; only the false-positive error logging is removed.