Skip to content

vregion: fixes to syscall validation - #11396

Open
kv2019i wants to merge 2 commits into
thesofproject:mainfrom
kv2019i:202611-vregion-syscall-val-fixes
Open

kv2019i wants to merge 2 commits into
thesofproject:mainfrom
kv2019i:202611-vregion-syscall-val-fixes

Conversation

@kv2019i

@kv2019i kv2019i commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

A few fixes to vregion syscall validation.

z_impl_vregion_create_map() wrote the region start and size to the
caller's buffers and then read them back to build the memory partition.
As a system call these buffers are user memory, so another thread could
change them in between and get arbitrary memory mapped into the LL
domain.

Read the requested size once, build the partitions from kernel locals
and write the results only at the end. Also set vr->domain after the
first partition is added, so a failure adding the second one unmaps the
first.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Copilot AI balanced review requested due to automatic review settings November 3, 2026 17:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Concurrent user-space vregion_put() calls can consume each other’s validator references, freeing the region before another release accesses it and causing a kernel use-after-free.

1 open issue
What changed in this PR

This PR hardens virtual-region syscall validation by checking handles against a kernel-maintained list and taking temporary references during syscall execution.

Changes:

  • Adds live-region tracking and reference-taking validation.
  • Updates syscall wrappers and the validation API.
  • Uses local mapping parameters and improves cleanup after partial mapping failures.
File Description
zephyr/​syscall/​vregion.c Uses reference-taking validation in syscall wrappers.
zephyr/​lib/​vregion.c Tracks live regions, updates lifetime handling, and hardens mapping setup.
src/​include/​sof/​lib/​vregion.h Renames the validator and documents its reference ownership contract.

Review effort: Balanced

Comment thread zephyr/syscall/vregion.c Outdated
Comment on lines +64 to +65
z_impl_vregion_put(vr);
return z_impl_vregion_put(vr);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in V2

@intel-sofci

intel-sofci commented Nov 3, 2026 •

Copy link
Copy Markdown

PR 11396: test results

Run date: 2026-11-04 12:51 UTC

Tested commit: 7740914ac5d80a6f3f024a46bb93b392c965d064

mtl pass rate lnl pass rate ptl pass rate wcl pass rate nvl pass rate

vregion_verify() accepted any pointer to memory the caller cannot read,
as long as the two words at its start looked like a user-writable range.
A forged handle let the kernel lock, count, initialize and free kernel
memory of the caller's choice. A concurrent vregion_put() could also free
a vregion between validation and use.

Keep live vregions in a kernel-only list. vregion_verify_pin() looks the
handle up and pins it under the list lock, and the validators unpin it
after the call. Pins are counted separately from references, so a
vregion_put() call of another thread can not consume them, and a
vregion is destroyed only when both its references and its pins are
gone. vregion_put() removes the last reference and the list entry
atomically with respect to lookups. The vregion_get() and vregion_put()
validators take or drop the caller's reference under the lock, and a
put without a reference left is rejected instead of underflowing the
count.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
kv2019i force-pushed the 202611-vregion-syscall-val-fixes branch from 33a7f58 to 7740914 Compare November 4, 2026 12:19
@kv2019i

kv2019i commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

V2 pushed:

  • address code review comments so far

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants