dma: add verify handling for all exported syscalls #96985
Conversation
Multiple dma.h functions are marked to be supported as syscalls, but don't have a verify handler, so attempt to use from user-space will fail. Add the missing verify handlers for dma_request_channel(), dma_release_channel(), dma_suspend() and dma_resume(). Signed-off-by: Kai Vehmanen <[email protected]>
| static int z_vrfy_dma_chan_filter(const struct device * dev, int channel, void * filter_param) | ||
| { | ||
| K_OOPS(K_SYSCALL_DRIVER_DMA(dev, chan_filter)); | ||
| return z_impl_dma_chan_filter((const struct device *)dev, channel, filter_param); |
There was a problem hiding this comment.
@teburd I marked this as draft for now, but wanted to share for some early comments.
I'm not 100% sure if this is enough to verify given filter param is a pointer. Most DMA drivers I checked just use the 'filter_param' to pass a value, so this would also work from user-space. But of course, if an actual pointer to something is passed, that won't work from user-space....
There was a problem hiding this comment.
This call should not be a syscall.
|
There was a problem hiding this comment.
I'm highly skeptical of any of these being safe as syscalls, but certainly not the one taking a void *. We don't know the size, we don't know the type, we don't even know if it should be read/write or read only based on the call parameter.
Typically this sort of thing we'd want to verify the address is valid, verify the size + address is permissable by the calling thread, copy the data into a local (kernel space) struct to avoid toctou, then do our work. We can't do anything like that with a void *.
chan_filter shouldn't be exposed to user mode in my view.
At a minimum start/stop/pause/resume should add some way of connecting the channel to the thread that owns them to make them safe. Otherwise a malicious or buggy usermode thread may start/stop/pause/resume a channel it does not own. I imagine this would be catastrophic in some cases.
Likely we need to create a new kobject to represent a dma channel to make this safe, so that ownership semantics are available. The fact that I made them syscalls now in retrospect seems quite wrong.
| static int z_vrfy_dma_chan_filter(const struct device * dev, int channel, void * filter_param) | ||
| { | ||
| K_OOPS(K_SYSCALL_DRIVER_DMA(dev, chan_filter)); | ||
| return z_impl_dma_chan_filter((const struct device *)dev, channel, filter_param); |
There was a problem hiding this comment.
This call should not be a syscall.
Multiple dma.h functions are marked to be supported as syscalls, but don't have a verify handler, so attempt to use from user-space will fail.
Add the missing verify handlers for dma_request_channel(), dma_release_channel(), dma_suspend() and dma_resume().