[PATCH] misc: fastrpc: reject overflowing invoke buffer ranges

[PATCH] misc: fastrpc: reject overflowing invoke buffer ranges

[Yousef Alhouseen]

fastrpc_get_buff_overlaps() builds end addresses from user ranges.

A wrapped end can understate the payload size.

It can also feed bad ranges into the invoke metadata.

Reject invoke buffers whose pointer plus length overflows.

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

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index f3a493845..ba4ade874 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of.h>
+#include <linux/overflow.h>
 #include <linux/platform_device.h>
 #include <linux/sort.h>
 #include <linux/of_platform.h>
@@ -607,14 +608,17 @@ static int olaps_cmp(const void *a, const void *b)
 	return st == 0 ? ed : st;
 }
 
-static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
+static int fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
 {
 	u64 max_end = 0;
 	int i;
 
 	for (i = 0; i < ctx->nbufs; ++i) {
 		ctx->olaps[i].start = ctx->args[i].ptr;
-		ctx->olaps[i].end = ctx->olaps[i].start + ctx->args[i].length;
+		if (check_add_overflow(ctx->olaps[i].start,
+				       ctx->args[i].length,
+				       &ctx->olaps[i].end))
+			return -EOVERFLOW;
 		ctx->olaps[i].raix = i;
 	}
 
@@ -641,6 +645,8 @@ static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
 			max_end = ctx->olaps[i].end;
 		}
 	}
+
+	return 0;
 }
 
 static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
@@ -675,7 +681,13 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
 			return ERR_PTR(-ENOMEM);
 		}
 		ctx->args = args;
-		fastrpc_get_buff_overlaps(ctx);
+		ret = fastrpc_get_buff_overlaps(ctx);
+		if (ret) {
+			kfree(ctx->olaps);
+			kfree(ctx->maps);
+			kfree(ctx);
+			return ERR_PTR(ret);
+		}
 	}
 
 	/* Released in fastrpc_context_put() */
-- 
2.54.0

Re: [PATCH] misc: fastrpc: reject overflowing invoke buffer ranges

[sashiko-bot]

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

Pre-existing issues:
- [Critical] The kernel trusts pointer and length values read directly from the DSP-shared DMA buffer, allowing an arbitrary kernel read/write vulnerability.
- [High] Context is published to the IDR before its refcount is initialized, allowing a concurrent DSP callback to trigger a Use-After-Free.
- [High] `cctx->rpdev` is read locklessly after an initial check, leading to a NULL pointer dereference if `fastrpc_rpmsg_remove` runs concurrently.
- [High] Integer truncation of `mlen` breaks buffer serialization and leads to DMA memory corruption.
--

commit 00821efcac58f87521dc0b1c609d5329a9afc752
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>

misc: fastrpc: reject overflowing invoke buffer ranges

This commit adds an overflow check to fastrpc_get_buff_overlaps(). It
verifies that adding the buffer length to its start address does not
overflow, returning -EOVERFLOW if it does. This prevents the use of
wrapped end addresses that could misstate payload sizes or feed invalid
ranges into invoke metadata.

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

[ ... ]

> @@ -675,7 +681,13 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
>  			return ERR_PTR(-ENOMEM);
>  		}
>  		ctx->args = args;
> -		fastrpc_get_buff_overlaps(ctx);
> +		ret = fastrpc_get_buff_overlaps(ctx);
> +		if (ret) {
> +			kfree(ctx->olaps);
> +			kfree(ctx->maps);
> +			kfree(ctx);
> +			return ERR_PTR(ret);
> +		}
>  	}
>  
>  	/* Released in fastrpc_context_put() */

[Severity: High]
This isn't a bug introduced by this patch, but since we are modifying
fastrpc_context_alloc(), could the context be published to the IDR
before it is fully initialized?

Further down in this function, the context is published:

	ret = idr_alloc_cyclic(&cctx->ctx_idr, ctx, 1,
			       FASTRPC_CTX_MAX, GFP_ATOMIC);

But the reference count is initialized later:

	kref_init(&ctx->refcount);

If an asynchronous callback like fastrpc_rpmsg_callback() retrieves this
newly published context from the IDR and drops its reference, could it
free the context before kref_init() executes, leading to a
use-after-free?


[Severity: High]
This is a pre-existing issue, but looking at fastrpc_internal_invoke(),
is there a lockless check that could lead to a NULL pointer dereference?

	if (!fl->cctx->rpdev)
		return -EPIPE;

If this check passes, and a concurrent device unbind clears the pointer
in fastrpc_rpmsg_remove() under a spinlock:

	spin_lock_irqsave(&cctx->lock, flags);
	cctx->rpdev = NULL;
	...
	spin_unlock_irqrestore(&cctx->lock, flags);

Could the invocation thread then dereference the NULL pointer when it
calls fastrpc_invoke_send()?

	ret = rpmsg_send(cctx->rpdev->ept, (void *)msg, sizeof(*msg));


[Severity: Critical]
This is a pre-existing issue, but does fastrpc_put_args() trust pointers
and lengths read directly from the DSP-shared DMA buffer without
validation?

In fastrpc_put_args():

			void *src = (void *)(uintptr_t)rpra[i].buf.pv;
			void *dst = (void *)(uintptr_t)ctx->args[i].ptr;
			u64 len = rpra[i].buf.len;

			if (!kernel) {
				if (copy_to_user((void __user *)dst, src, len)) {

If a compromised DSP supplies arbitrary addresses in this shared region,
could this result in an arbitrary kernel read or write?


[Severity: High]
This is a pre-existing issue, but does fastrpc_get_args() truncate the
64-bit overlap size into a 32-bit signed integer?

	for (oix = 0; oix < ctx->nbufs; ++oix) {
		int mlen;
		...
			mlen = ctx->olaps[oix].mend - ctx->olaps[oix].mstart;

			if (rlen < mlen)
				goto bail;

			rpra[i].buf.pv = args - ctx->olaps[oix].offset;
			...
			args = args + mlen;

If the overlap exceeds 2GB, mlen could truncate to a negative value or a
small positive value. Could this cause unsigned promotion to erroneously
reject the buffer, or fail to advance the payload pointer sufficiently,
overwriting subsequent buffers?

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

Re: [PATCH] misc: fastrpc: reject overflowing invoke buffer ranges

[Srinivas Kandagatla]

On 6/24/26 6:44 PM, Yousef Alhouseen wrote:
> fastrpc_get_buff_overlaps() builds end addresses from user ranges.
> 
> A wrapped end can understate the payload size.
> 
> It can also feed bad ranges into the invoke metadata.
> 
> Reject invoke buffers whose pointer plus length overflows.
> 
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
You have sent 11 patches independently, I would prefer it to be sent as
single series.

Are these patches fixing anything that your usecases are hitting?

Have you looked at the patches in the mailing list which fixes some of
these issues?

Or

Is AI generating these patches ?


--srini


> ---
>  drivers/misc/fastrpc.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index f3a493845..ba4ade874 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -13,6 +13,7 @@
>  #include <linux/module.h>
>  #include <linux/of_address.h>
>  #include <linux/of.h>
> +#include <linux/overflow.h>
>  #include <linux/platform_device.h>
>  #include <linux/sort.h>
>  #include <linux/of_platform.h>
> @@ -607,14 +608,17 @@ static int olaps_cmp(const void *a, const void *b)
>  	return st == 0 ? ed : st;
>  }
>  
> -static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
> +static int fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
>  {
>  	u64 max_end = 0;
>  	int i;
>  
>  	for (i = 0; i < ctx->nbufs; ++i) {
>  		ctx->olaps[i].start = ctx->args[i].ptr;
> -		ctx->olaps[i].end = ctx->olaps[i].start + ctx->args[i].length;
> +		if (check_add_overflow(ctx->olaps[i].start,
> +				       ctx->args[i].length,
> +				       &ctx->olaps[i].end))
> +			return -EOVERFLOW;
>  		ctx->olaps[i].raix = i;
>  	}
>  
> @@ -641,6 +645,8 @@ static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
>  			max_end = ctx->olaps[i].end;
>  		}
>  	}
> +
> +	return 0;
>  }
>  
>  static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
> @@ -675,7 +681,13 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
>  			return ERR_PTR(-ENOMEM);
>  		}
>  		ctx->args = args;
> -		fastrpc_get_buff_overlaps(ctx);
> +		ret = fastrpc_get_buff_overlaps(ctx);
> +		if (ret) {
> +			kfree(ctx->olaps);
> +			kfree(ctx->maps);
> +			kfree(ctx);
> +			return ERR_PTR(ret);
> +		}
>  	}
>  
>  	/* Released in fastrpc_context_put() */

Re: [PATCH] misc: fastrpc: reject overflowing invoke buffer ranges

[Yousef Alhouseen]

Hi Srini,

These are not fixes from a FastRPC use case I am running, and I do not
have FastRPC hardware for runtime testing. They came from static code
review. I used an AI coding assistant during discovery and drafting,
then reviewed the changes and ran strict checkpatch and focused
compile checks before sending them.

I had not completed a proper reconciliation against the current
mailing-list work before sending the patches independently. That, and
sending so many related changes as separate threads, was a process
mistake.

I am pausing this set. I will first compare every item with the
existing list series, drop duplicates and weak findings, and re-audit
the lifetime changes in light of the reported concurrency concerns. I
will only return with a small ordered series if anything remains
defensible, with the testing limits stated explicitly.

Thanks,
Yousef

On Wed, 1 Jul 2026 21:05:37 +0100, Srinivas Kandagatla <srini@kernel.org> wrote:
> On 6/24/26 6:44 PM, Yousef Alhouseen wrote:
> > fastrpc_get_buff_overlaps() builds end addresses from user ranges.
> >
> > A wrapped end can understate the payload size.
> >
> > It can also feed bad ranges into the invoke metadata.
> >
> > Reject invoke buffers whose pointer plus length overflows.
> >
> > Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> You have sent 11 patches independently, I would prefer it to be sent as
> single series.
>
> Are these patches fixing anything that your usecases are hitting?
>
> Have you looked at the patches in the mailing list which fixes some of
> these issues?
>
> Or
>
> Is AI generating these patches ?
>
> --srini
>
> > ---
> > drivers/misc/fastrpc.c | 18 +++++++++++++++---
> > 1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > index f3a493845..ba4ade874 100644
> > --- a/drivers/misc/fastrpc.c
> > +++ b/drivers/misc/fastrpc.c
> > @@ -13,6 +13,7 @@
> > #include <linux/module.h>
> > #include <linux/of_address.h>
> > #include <linux/of.h>
> > +#include <linux/overflow.h>
> > #include <linux/platform_device.h>
> > #include <linux/sort.h>
> > #include <linux/of_platform.h>
> > @@ -607,14 +608,17 @@ static int olaps_cmp(const void *a, const void *b)
> > return st == 0 ? ed : st;
> > }
> >
> > -static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
> > +static int fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
> > {
> > u64 max_end = 0;
> > int i;
> >
> > for (i = 0; i < ctx->nbufs; ++i) {
> > ctx->olaps[i].start = ctx->args[i].ptr;
> > - ctx->olaps[i].end = ctx->olaps[i].start + ctx->args[i].length;
> > + if (check_add_overflow(ctx->olaps[i].start,
> > + ctx->args[i].length,
> > + &ctx->olaps[i].end))
> > + return -EOVERFLOW;
> > ctx->olaps[i].raix = i;
> > }
> >
> > @@ -641,6 +645,8 @@ static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
> > max_end = ctx->olaps[i].end;
> > }
> > }
> > +
> > + return 0;
> > }
> >
> > static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
> > @@ -675,7 +681,13 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
> > return ERR_PTR(-ENOMEM);
> > }
> > ctx->args = args;
> > - fastrpc_get_buff_overlaps(ctx);
> > + ret = fastrpc_get_buff_overlaps(ctx);
> > + if (ret) {
> > + kfree(ctx->olaps);
> > + kfree(ctx->maps);
> > + kfree(ctx);
> > + return ERR_PTR(ret);
> > + }
> > }
> >
> > /* Released in fastrpc_context_put() */