Skip to content

[userspace LL] vregion related syscalls - #11108

Open
lyakh wants to merge 6 commits into
thesofproject:mainfrom
lyakh:vreg
Open

[userspace LL] vregion related syscalls#11108
lyakh wants to merge 6 commits into
thesofproject:mainfrom
lyakh:vreg

Conversation

@lyakh

@lyakh lyakh commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

vregion system calls, needed when integrating userspace LL and DP
part of #10945

Copilot AI lite review requested due to automatic review settings August 20, 2026 14:40

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.

Pull request overview

Adds Zephyr userspace support for SOF “vregion” allocations to enable userspace LL + DP integration (part of #10945), including syscall plumbing and module-adapter wiring to map vregion memory into the LL userspace memory domain.

Changes:

  • Introduces Zephyr syscall handlers/marshalling for vregion alloc/free/get/put/set_interim.
  • Refactors Zephyr vregion implementation entrypoints to z_impl_* to back the new syscalls.
  • Extends module-adapter allocation flow to create/map/unmap vregions for DP modules and plumbs vregion start/size through mod_alloc_ctx.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
zephyr/syscall/vregion.c New syscall verification + marshalling includes for vregion APIs.
zephyr/lib/vregion.c Switches vregion APIs to z_impl_* entrypoints and adjusts symbol exports accordingly.
zephyr/Kconfig Adds SOF_USERSPACE_INTERFACE_VREGION and selects it from SOF_USERSPACE_LL.
zephyr/include/rtos/alloc.h Extends mod_alloc_ctx with vregion start/size metadata for domain mapping.
zephyr/CMakeLists.txt Adds syscall header generation and builds the new vregion syscall source.
src/include/sof/lib/vregion.h Marks vregion APIs as __syscall and includes generated syscall header.
src/include/sof/audio/module_adapter/module/generic.h Exposes module-adapter vregion map/unmap as syscalls for full Zephyr app.
src/audio/module_adapter/module_adapter.c Implements vregion creation + mem-domain partition mapping and adds syscall verifiers.
src/audio/buffers/comp_buffer.c Routes vregion-backed buffer free through the new shared vregion-free helper.
Suppressed comments (2)

src/audio/module_adapter/module_adapter.c:134

  • module_adapter_vreg_free() decrements the vregion refcount (and may free the vregion pages) before removing the user mem-domain partitions. If vregion_put() frees the pages, the user partition remains until module_adapter_vreg_unmap() runs, creating a window where freed (and potentially reallocated) pages stay user-accessible. Consider unmapping first and freeing the vregion atomically in kernel code when the refcount reaches 0.
void module_adapter_vreg_free(struct mod_alloc_ctx *alloc)
{
	if (vregion_put(alloc->vreg))
		return;

	module_adapter_vreg_unmap(alloc);

	sof_heap_free(alloc->heap, alloc);

src/audio/module_adapter/module_adapter.c:255

  • The syscall verifier for module_adapter_vreg_unmap() only checks that the alloc struct is readable. In userspace-LL configurations alloc lives in user-writable memory, so a caller can forge vreg_start/vreg_size and attempt to remove arbitrary partitions from zephyr_ll_mem_domain(). Add validation that alloc refers to an expected allocation and that (vreg_start,vreg_size) match the vregion’s actual mem_info (or avoid taking alloc from user-space entirely).
void z_vrfy_module_adapter_vreg_unmap(const struct mod_alloc_ctx *alloc)
{
	K_OOPS(K_SYSCALL_MEMORY_READ(alloc, sizeof(*alloc)));
	z_impl_module_adapter_vreg_unmap(alloc);
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/audio/module_adapter/module_adapter.c Outdated
Comment thread src/audio/module_adapter/module_adapter.c Outdated
Comment thread zephyr/syscall/vregion.c Outdated
Comment thread src/include/sof/lib/vregion.h
Comment thread src/include/sof/audio/module_adapter/module/generic.h Outdated
Comment thread zephyr/CMakeLists.txt Outdated

@kv2019i kv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please check inline, concern with the syscall verify function.

void z_vrfy_module_adapter_vreg_unmap(const struct mod_alloc_ctx *alloc)
{
K_OOPS(K_SYSCALL_MEMORY_READ(alloc, sizeof(*alloc)));
z_impl_module_adapter_vreg_unmap(alloc);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm, this is not safe, user-space can pass arbitrary alloc->vreg and this is passed unchecked to kernel code.

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.

@kv2019i added an owner, please re-check

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is better, but as "alloc" is untrusted memory, "alloc->vreg" can point to a made-up vreg object (with owner set to point to alloc by the user-space thread). I think another layer is needed to look up "vreg" is an actual kernel vreg object, and not a pointer to some random user created object.

@lyakh
lyakh force-pushed the vreg branch 2 times, most recently from 6f6c216 to c223bae Compare August 24, 2026 10:59
@lyakh
lyakh force-pushed the vreg branch 2 times, most recently from c5e4761 to 88da26b Compare August 24, 2026 11:59
lyakh and others added 4 commits August 24, 2026 14:21
Add two syscall functions to allocate and map, and to unmap vregion
for userspace modules. For now only used for DP modules. Add
module_adapter.c to cmocka builds for the new
module_adapter_vreg_free() function, which is now called in
comp_buffer.c.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
scheduler_dp_task_init() currently only runs in privileged mode, add
a comment and a check for that.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
The entire user_access_to_mailbox() function is already under
an #ifdef CONFIG_SOF_USERSPACE_LL condition. Remove an additional
identical check inside the function.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Make vregion_alloc(), vregion_alloc_coherent(),
vregion_alloc_align(), vregion_alloc_coherent_align(),
and vregion_free() available as Zephyr system calls for
user-space threads.

Add K_SYSCALL_MEMORY_WRITE verification to all syscall
handlers to validate the calling thread has access to
the vregion's managed memory area.

Add CONFIG_SOF_USERSPACE_INTERFACE_VREGION Kconfig option
to control the feature. It is auto-selected by
SOF_USERSPACE_LL when SOF_VREGIONS is enabled.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
lyakh added 2 commits August 24, 2026 15:39
Extract common syscall verification code into a function.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
vregion_get(), vregion_put() and vregion_set_interim() should also be
callable from the userspace. Make them syscalls. Also remove
redundant symbol exporting since the vregion API shouldn't be used
directly by LLEXT modules.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

@kv2019i kv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The new check in vregion_verify() could be enough, please see comments inline. At least I'd add a comment how this protects against invalid/fabricated "vr" objects.

void z_vrfy_module_adapter_vreg_unmap(const struct mod_alloc_ctx *alloc)
{
K_OOPS(K_SYSCALL_MEMORY_READ(alloc, sizeof(*alloc)));
z_impl_module_adapter_vreg_unmap(alloc);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is better, but as "alloc" is untrusted memory, "alloc->vreg" can point to a made-up vreg object (with owner set to point to alloc by the user-space thread). I think another layer is needed to look up "vreg" is an actual kernel vreg object, and not a pointer to some random user created object.

Comment thread zephyr/syscall/vregion.c Outdated
size_t vr_size = 0;
uintptr_t vr_start;

vregion_mem_info(vr, &vr_size, &vr_start);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think here too we'd need to verify "vr" is a valid kernel vr object.

Comment thread zephyr/syscall/vregion.c
size_t vr_size = 0;
uintptr_t vr_start;
if (vregion_verify(vr))
return z_impl_vregion_alloc(vr, size);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Commit description says you put common code into vregion_verify(), but it actually does more checks than before, so this is not just about putting it in common function.

Comment thread zephyr/lib/vregion.c
return false;

/* vregion instances must not be accessible to the userspace. */
K_OOPS(!K_SYSCALL_MEMORY_READ(vr, sizeof(*vr)));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ok this could be potentially enough. I still wonder if this is secure enough. but this is definitely a fast check to make, versus looking up a list/array of all kernel vr objects.

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