On Tue, Mar 24, 2026 at 07:45:14PM +0800, Pengpeng Hou wrote:
> fastrpc_get_args() derives rpra[i].buf.pv from the overlap offset that
> was computed from user-controlled argument pointers and lengths. The
> resulting destination pointer is later passed to copy_from_user() or
> memcpy() without checking that the overlap-adjusted range still stays
> inside the allocated invoke buffer.
>
> Reject overlap-derived destinations that would point before the start of
> the invoke buffer or that would extend past the end of the allocated
> packet before storing rpra[i].buf.pv and before copying inline
> arguments into the buffer.
>
> Fixes: c68cfb718c8f ("misc: fastrpc: Add support for context Invoke method")
> Found by static code analysis.
Please don't mix tags with the free flow text. Also please don't send
next iterations as a response to your previous email. Always start a new
thread for the new revision.
> Signed-off-by: Pengpeng Hou <[email protected]>
> ---
> v2:
> - add the missing Signed-off-by line
> - add a Fixes tag and note that the issue was found by static code analysis
> - run checkpatch and keep the patch checkpatch-clean
>
> drivers/misc/fastrpc.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 47356a5d5804..7dfb5eb6dc92 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -993,6 +993,7 @@ static int fastrpc_get_args(u32 kernel, struct
> fastrpc_invoke_ctx *ctx)
> u64 len, rlen, pkt_size;
> u64 pg_start, pg_end;
> uintptr_t args;
> + uintptr_t buf_start, buf_end;
> int metalen;
>
> inbufs = REMOTE_SCALARS_INBUFS(ctx->sc);
> @@ -1016,6 +1017,8 @@ static int fastrpc_get_args(u32 kernel, struct
> fastrpc_invoke_ctx *ctx)
> rpra = ctx->buf->virt;
> list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
> pages = fastrpc_phy_page_start(list, ctx->nscalars);
> + buf_start = (uintptr_t)ctx->buf->virt;
> + buf_end = buf_start + pkt_size;
> args = (uintptr_t)ctx->buf->virt + metalen;
> rlen = pkt_size - metalen;
> ctx->rpra = rpra;
> @@ -1053,6 +1056,7 @@ static int fastrpc_get_args(u32 kernel, struct
> fastrpc_invoke_ctx *ctx)
> pages[i].size = (pg_end - pg_start + 1) * PAGE_SIZE;
>
> } else {
> + uintptr_t dst;
>
> if (ctx->olaps[oix].offset == 0) {
> rlen -= ALIGN(args, FASTRPC_ALIGN) - args;
> @@ -1064,7 +1068,18 @@ static int fastrpc_get_args(u32 kernel, struct
> fastrpc_invoke_ctx *ctx)
> if (rlen < mlen)
> goto bail;
>
> - rpra[i].buf.pv = args - ctx->olaps[oix].offset;
> + if (ctx->olaps[oix].offset > args - buf_start) {
> + err = -EINVAL;
> + goto bail;
> + }
if (buf_start + ctx->olaps[oix].offset > args)?
This needs some more explanation anyway...
> +
> + dst = args - ctx->olaps[oix].offset;
> + if (len > buf_end - dst) {
Can this overflow?
> + err = -EINVAL;
> + goto bail;
> + }
> +
> + rpra[i].buf.pv = dst;
> pages[i].addr = ctx->buf->dma_addr -
> ctx->olaps[oix].offset +
> (pkt_size - rlen);
> --
> 2.50.1 (Apple Git-155)
>
--
With best wishes
Dmitry
