[PATCH] misc: fastrpc: exclude mapped buffers from inline overlaps

[PATCH] misc: fastrpc: exclude mapped buffers from inline overlaps

[Yousef Alhouseen]

fastrpc_get_buff_overlaps() computes overlap offsets for every invoke
buffer, but only buffers without an fd are serialized into the inline DMA
payload. A mapped buffer can therefore advance max_end without reserving
inline payload space.

If a following inline buffer overlaps that mapped range, the serializer
uses the mapped range to derive a negative offset into the inline buffer
and then copies user data there. Ignore mapped buffers when building the
inline overlap state so inline payload pointers always refer to memory
that was actually reserved in the invoke buffer.

Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
 drivers/misc/fastrpc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index ed0041076..1b70acc10 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -630,6 +630,15 @@ static int fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
 	sort(ctx->olaps, ctx->nbufs, sizeof(*ctx->olaps), olaps_cmp, NULL);
 
 	for (i = 0; i < ctx->nbufs; ++i) {
+		int raix = ctx->olaps[i].raix;
+
+		if (ctx->args[raix].fd != 0 && ctx->args[raix].fd != -1) {
+			ctx->olaps[i].mstart = 0;
+			ctx->olaps[i].mend = 0;
+			ctx->olaps[i].offset = 0;
+			continue;
+		}
+
 		/* Falling inside previous range */
 		if (ctx->olaps[i].start < max_end) {
 			ctx->olaps[i].mstart = max_end;
-- 
2.54.0

Re: [PATCH] misc: fastrpc: exclude mapped buffers from inline overlaps

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Integer truncation in `__fastrpc_buf_alloc` allows out-of-bounds DMA access.
- [Critical] Use-After-Free and Double Free in `fastrpc_map_attach` error path.
- [Critical] Integer truncation of `mlen` allows kernel buffer overflow via `copy_from_user`.
- [High] Memory leak of `struct fastrpc_map` for handles during context cleanup.
- [High] Circular reference leak when `fastrpc_internal_invoke` is interrupted.
--

commit a1f9ba556dd9f43a1a5e5fbad752e8ca90d62449
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
Subject: misc: fastrpc: exclude mapped buffers from inline overlaps

This commit modifies fastrpc_get_buff_overlaps() to ignore mapped buffers when
computing overlap offsets. It ensures that only buffers without a file
descriptor are serialized into the inline DMA payload. This prevents the
serializer from deriving negative offsets and writing to unreserved memory.

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index f3a49384586d1..79b428addf062 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c

[ ... ]

[Severity: Critical]
This is a pre-existing issue, but does passing the 64-bit buf->size to the 
size_t argument of dma_alloc_coherent() result in truncation on 32-bit 
architectures?

drivers/misc/fastrpc.c:__fastrpc_buf_alloc() {
    ...
    buf->virt = dma_alloc_coherent(dev, buf->size, &buf->dma_addr, GFP_KERNEL);
    ...
}

Could this cause an out-of-bounds DMA access when the untruncated 64-bit size 
is sent to the DSP here?

drivers/misc/fastrpc.c:fastrpc_invoke_send() {
    ...
    msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
    ...
}

[Severity: High]
This is a pre-existing issue, but does this allocation lack a corresponding 
cleanup path for handles?

drivers/misc/fastrpc.c:fastrpc_create_maps() {
    ...
    else
        err = fastrpc_map_attach(ctx->fl, ctx->args[i].fd, ctx->args[i].length, ctx->args[i].attr, &ctx->maps[i]);
    ...
}

The cleanup loop in fastrpc_context_free() terminates prematurely at ctx->nbufs:

drivers/misc/fastrpc.c:fastrpc_context_free() {
    ...
    for (i = 0; i < ctx->nbufs; i++)
        fastrpc_map_put(ctx->maps[i]);
    ...
}

Does this fail to drop references for mappings populated up to ctx->nscalars, 
resulting in a memory leak of struct fastrpc_map?

[Severity: High]
This is a pre-existing issue, but does this create a circular reference if the 
context is not freed?

drivers/misc/fastrpc.c:fastrpc_context_alloc() {
    ...
    /* Take a reference to user, released in fastrpc_context_free() */
    fastrpc_user_get(user);
    ...
}

Looking at the error path for -ERESTARTSYS, it bypasses context cleanup:

drivers/misc/fastrpc.c:fastrpc_internal_invoke() {
    ...
    if (err != -ERESTARTSYS && err != -ETIMEDOUT) {
        spin_lock(&fl->lock);
        list_del(&ctx->node);
        spin_unlock(&fl->lock);
        fastrpc_context_put(ctx);
    }
    ...
}

If interrupted, does this strand the context with its elevated refcount and leak 
the fastrpc_invoke_ctx and fastrpc_user resources?

[Severity: Critical]
This is a pre-existing issue, but the error path manually detaches and puts the 
buffer, then drops the map reference:

drivers/misc/fastrpc.c:fastrpc_map_attach() {
    ...
map_err:
    dma_buf_detach(map->buf, map->attach);
attach_err:
    dma_buf_put(map->buf);
get_err:
    fastrpc_map_put(map);
    ...
}

Does fastrpc_free_map() then execute its own cleanup block because map->table 
is non-NULL?

drivers/misc/fastrpc.c:fastrpc_free_map() {
    ...
    if (map->table) {
        // ...
        dma_buf_unmap_attachment_unlocked(map->attach, map->table, DMA_BIDIRECTIONAL);
        dma_buf_detach(map->buf, map->attach);
        dma_buf_put(map->buf);
    }
    ...
}

Could this lead to a use-after-free and double free of the dma_buf_attachment?

[Severity: Critical]
This is a pre-existing issue, but does assigning the 64-bit difference to the 
32-bit int mlen silently truncate the value?

drivers/misc/fastrpc.c:fastrpc_get_args() {
    ...
    for (oix = 0; oix < ctx->nbufs; ++oix) {
        int mlen;
        // ...
        mlen = ctx->olaps[oix].mend - ctx->olaps[oix].mstart;
    ...
}

Can this truncation cause validation checks to fail and pointer arithmetic to 
underflow, allowing an out-of-bounds write during copy_from_user?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624192309.4723-1-alhouseenyousef@gmail.com?part=1