[PATCH] misc: fastrpc: keep copied arguments inside the invoke buffer

[PATCH] misc: fastrpc: keep copied arguments inside the invoke buffer

[Pengpeng Hou]

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 then used for copy_from_user() without
first checking that it still falls inside the allocated invoke buffer.

Validate the overlap-derived destination range before storing it in
rpra[i].buf.pv and before copying inline arguments into the invoke
buffer.
---
 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..d7f53300b899 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;
+			}
+
+			dst = args - ctx->olaps[oix].offset;
+			if (len > buf_end - dst) {
+				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)

Re: [PATCH] misc: fastrpc: keep copied arguments inside the invoke buffer

[Konrad Dybcio]

On 3/24/26 2:44 AM, 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 then used for copy_from_user() without
> first checking that it still falls inside the allocated invoke buffer.
> 
> Validate the overlap-derived destination range before storing it in
> rpra[i].buf.pv and before copying inline arguments into the invoke
> buffer.
> ---

Your contribution lacks a DCO:

https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

without which we can't accept it.

Please run ./scripts/checkpatch.pl on the patch file

Konrad

Re: [PATCH] misc: fastrpc: keep copied arguments inside the invoke buffer

[Pengpeng Hou]

Hi Konrad,

Thanks for pointing that out.

I have added the missing Signed-off-by line, ran checkpatch on the patch
file, and prepared a cleaned up v2.

Best regards,
Pengpeng

[PATCH v2] misc: fastrpc: validate overlap-derived invoke buffer ranges

[Pengpeng Hou]

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.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
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;
+			}
+
+			dst = args - ctx->olaps[oix].offset;
+			if (len > buf_end - dst) {
+				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)

Re: [PATCH v2] misc: fastrpc: validate overlap-derived invoke buffer ranges

[Dmitry Baryshkov]

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 <pengpeng@iscas.ac.cn>
> ---
> 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