Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The syscall lacks userspace buffer validation and introduces problematic singleton blob-handler semantics.
Review effort: Balanced
Findings: 2
Open (3)
What changed in this PR
Adds userspace access to module data-blob configuration, supporting DP modules such as microwakeword.
Changes:
- Adds
mod_data_blob_set()syscall and verifier. - Reuses tracked blob handlers.
- Moves zero-size allocation rejection before locking.
| File | Description |
|---|---|
generic.h |
Declares the data-blob syscall API. |
generic.c |
Implements blob lookup, setting, verification, and allocation adjustment. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Checking for size == 0 can be done in the beginning of z_impl_mod_alloc_ext() to simplify clean up. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Modules, running in the userspace cannot call comp_data_blob_set() directly. Add a module wrapper syscall for it. This also performs the same access rights check as other module syscalls. Also enforce a single data blob per module. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Spotted one problem, but it's not specific to this PR, so needs to be handled in a follow-up.
| struct module_resource *container; | ||
|
|
||
| k_mutex_lock(&mod_res_lock, K_FOREVER); | ||
| int ret = objpool_iterate(&res->objpool, mod_data_blob_find, &bhp); |
There was a problem hiding this comment.
Not a new issue with this PR, but now realize this is not safe. objpool_iterate will go through pointers that may not be safe and z_vrfy_mod_data_blob_set() won't cover this.
This continues work in #11199 which in 78d5c92 enabled using data blobs in module context, but to use it in userspace an additional syscall is needed. This is needed for #11135