Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Critical] fastrpc: DMA use-after-free by DSP due to missed 
invoke_interrupted_mmaps transfer
--

commit 77b5628f93f366d6c2407907756d94941e50a6f7
Author: Yao Yiqi <[email protected]>

misc: fastrpc: Fix double free and races in fastrpc_req_mmap() error path

This commit defers adding the newly allocated buffer to the fl->mmaps list
until after copy_to_user() succeeds. If the copy fails and the subsequent
unmap operation also fails, the buffer is re-added to the list to prevent
a leak.

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
[ ... ]
> @@ -2153,22 +2153,26 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, 
> char __user *argp)
[ ... ]
>  err_assign:
> -     fastrpc_req_munmap_impl(fl, buf);
> +     if (fastrpc_req_munmap_impl(fl, buf)) {
> +             spin_lock(&fl->lock);
> +             list_add_tail(&buf->node, &fl->mmaps);
> +             spin_unlock(&fl->lock);
> +     }
>  
>       return err;
>  }

[Severity: Critical]
Is there a risk of a DMA use-after-free if the unmap operation is
interrupted by a signal?

When fastrpc_req_munmap_impl() is interrupted, fastrpc_internal_invoke()
handles this by moving the buffer to the invoke_interrupted_mmaps list to
intentionally leak it and prevent the DSP from accessing freed memory:

drivers/misc/fastrpc.c:fastrpc_internal_invoke() {
    ...
    if (err == -ERESTARTSYS) {
        spin_lock(&fl->lock);
        list_for_each_entry_safe(buf, b, &fl->mmaps, node) {
            list_del(&buf->node);
            list_add_tail(&buf->node, &fl->cctx->invoke_interrupted_mmaps);
        }
        spin_unlock(&fl->lock);
    }
    ...
}

Because this patch defers adding the buffer to fl->mmaps until after
copy_to_user(), fastrpc_internal_invoke() will not find the buffer in
fl->mmaps when called from this error path, missing the transfer to the
interrupted list.

If fastrpc_req_munmap_impl() returns -ERESTARTSYS, this err_assign path
then manually adds the buffer back to fl->mmaps. When the process later
exits, fastrpc_user_free() will reclaim this buffer while the DSP may still
be using it, which could corrupt newly allocated kernel memory.

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to