zephyr: syscall: sof_dma: fixes to dma_config syscall verification#10873
zephyr: syscall: sof_dma: fixes to dma_config syscall verification#10873kv2019i wants to merge 3 commits into
Conversation
…st() deep_copy_dma_blk_cfg_list() is used to verify the syscall arguments. Fix an issue with possible overflow when calculating the alloc size for DMA blocks. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
requested review from
abonislawski,
dbaluta,
iuliana-prodan and
lyakh
as code owners
There was a problem hiding this comment.
Pull request overview
This PR hardens the Zephyr userspace syscall validation path for SOF DMA configuration by preventing integer overflow when allocating the kernel-side copy of user-provided DMA block configurations.
Changes:
- Add
size_mul_overflow()-based overflow detection when computing the allocation size fordma_block_configarrays derived from user-controlledblock_count. - Use the checked
alloc_sizewhen callingrmalloc().
z_vrfy_sof_dma_config() only passes allowed fields of dma_config struct to kernel. Add initialization of the stack dma_config object kern_cfg to ensure the skipped fields are set to a known state. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
deep_copy_dma_blk_cfg_list() does not check that number of entries in the linked list of DMA blocks matches cfg->block_count. This could be used to make kernel read from unvalidated user memory. Fix the issue by limiting list traversal to cfg->block_count. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
changed the title
|
V2:
|
| kern_next->next_block = NULL; | ||
| else | ||
| goto err; | ||
| } |
There was a problem hiding this comment.
you could remove the if and just put the else block after the loop? Or keep the if to stay correct and avoid a redundant assignment and just move the else out
There was a problem hiding this comment.
@lyakh worth it? this is not performance critical, so I'd rather keep the logic in the same place inside the for loop. Now it's easy to see that the unsafe "kern_next_block" is overwritten in all cases. It's run only once in either case. If more votes, I can move to outside for loop, agreed it will be functionally the same.
Series of fixes to sof_dma dma_config syscall verification.
Note that this interface is not yet enabled in any product configuration, so exposure is currently limited.