[PATCH] optee: Check return value of tee_shm_get_va()

[PATCH] optee: Check return value of tee_shm_get_va()

[Chen Ni]

The function tee_shm_get_va() can return an error pointer if the shared
memory is not properly mapped or if the offset is invalid. Without this
check, passing the error pointer to subsequent memory operations could
lead to a kernel panic.

Add a check for IS_ERR() on the return value of tee_shm_get_va().

Fixes: f0c8431568ee ("optee: probe RPMB device using RPMB subsystem")
Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
---
 drivers/tee/optee/rpc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
index b0ed4cb49452..32f7742c094c 100644
--- a/drivers/tee/optee/rpc.c
+++ b/drivers/tee/optee/rpc.c
@@ -393,6 +393,11 @@ static void handle_rpc_func_rpmb_frames(struct tee_context *ctx,
 			    params[0].u.memref.shm_offs);
 	p1 = tee_shm_get_va(params[1].u.memref.shm,
 			    params[1].u.memref.shm_offs);
+	if (IS_ERR(p0) || IS_ERR(p1)) {
+		arg->ret = TEEC_ERROR_BAD_PARAMETERS;
+		goto out;
+	}
+
 	if (rpmb_route_frames(rdev, p0, params[0].u.memref.size, p1,
 			      params[1].u.memref.size)) {
 		arg->ret = TEEC_ERROR_BAD_PARAMETERS;
-- 
2.25.1

Re: [PATCH] optee: Check return value of tee_shm_get_va()

[Markus Elfring via OP-TEE]

…
> +++ b/drivers/tee/optee/rpc.c
> @@ -393,6 +393,11 @@ static void handle_rpc_func_rpmb_frames(struct tee_context *ctx,
>  			    params[0].u.memref.shm_offs);
>  	p1 = tee_shm_get_va(params[1].u.memref.shm,
>  			    params[1].u.memref.shm_offs);
> +	if (IS_ERR(p0) || IS_ERR(p1)) {
> +		arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> +		goto out;
> +	}
> +
>  	if (rpmb_route_frames(rdev, p0, params[0].u.memref.size, p1,
>  			      params[1].u.memref.size)) {
>  		arg->ret = TEEC_ERROR_BAD_PARAMETERS;
…

How do you think about to use an additional label for the shown
error code assignment?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v7.0-rc2#n526

Regards,
Markus

Re: [PATCH] optee: Check return value of tee_shm_get_va()

[Sumit Garg via OP-TEE]

On Fri, Mar 06, 2026 at 12:52:39PM +0100, Markus Elfring wrote:
> …
> > +++ b/drivers/tee/optee/rpc.c
> > @@ -393,6 +393,11 @@ static void handle_rpc_func_rpmb_frames(struct tee_context *ctx,
> >  			    params[0].u.memref.shm_offs);
> >  	p1 = tee_shm_get_va(params[1].u.memref.shm,
> >  			    params[1].u.memref.shm_offs);
> > +	if (IS_ERR(p0) || IS_ERR(p1)) {
> > +		arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > +		goto out;
> > +	}
> > +
> >  	if (rpmb_route_frames(rdev, p0, params[0].u.memref.size, p1,
> >  			      params[1].u.memref.size)) {
> >  		arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> …
> 
> How do you think about to use an additional label for the shown
> error code assignment?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v7.0-rc2#n526
>

I suppose here you meant to update the label name since it's the same
error type used by other code paths too. So following label rename
should be fine I think as per coding guidelines:

s/out/err_dev_put/

-Sumit

Re: [PATCH] optee: Check return value of tee_shm_get_va()

[Jens Wiklander]

On Thu, Mar 12, 2026 at 10:16 AM Sumit Garg <sumit.garg@kernel.org> wrote:
>
> On Fri, Mar 06, 2026 at 12:52:39PM +0100, Markus Elfring wrote:
> > …
> > > +++ b/drivers/tee/optee/rpc.c
> > > @@ -393,6 +393,11 @@ static void handle_rpc_func_rpmb_frames(struct tee_context *ctx,
> > >                         params[0].u.memref.shm_offs);
> > >     p1 = tee_shm_get_va(params[1].u.memref.shm,
> > >                         params[1].u.memref.shm_offs);
> > > +   if (IS_ERR(p0) || IS_ERR(p1)) {
> > > +           arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > +           goto out;
> > > +   }
> > > +
> > >     if (rpmb_route_frames(rdev, p0, params[0].u.memref.size, p1,
> > >                           params[1].u.memref.size)) {
> > >             arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > …
> >
> > How do you think about to use an additional label for the shown
> > error code assignment?
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v7.0-rc2#n526
> >
>
> I suppose here you meant to update the label name since it's the same
> error type used by other code paths too. So following label rename
> should be fine I think as per coding guidelines:
>
> s/out/err_dev_put/

Wouldn't the name err_dev_put suggest this only occurs in the error path?

Cheers,
Jens

>
> -Sumit

Re: [PATCH] optee: Check return value of tee_shm_get_va()

[Sumit Garg via OP-TEE]

On Thu, Mar 12, 2026 at 12:16:11PM +0100, Jens Wiklander wrote:
> On Thu, Mar 12, 2026 at 10:16 AM Sumit Garg <sumit.garg@kernel.org> wrote:
> >
> > On Fri, Mar 06, 2026 at 12:52:39PM +0100, Markus Elfring wrote:
> > > …
> > > > +++ b/drivers/tee/optee/rpc.c
> > > > @@ -393,6 +393,11 @@ static void handle_rpc_func_rpmb_frames(struct tee_context *ctx,
> > > >                         params[0].u.memref.shm_offs);
> > > >     p1 = tee_shm_get_va(params[1].u.memref.shm,
> > > >                         params[1].u.memref.shm_offs);
> > > > +   if (IS_ERR(p0) || IS_ERR(p1)) {
> > > > +           arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > > +           goto out;
> > > > +   }
> > > > +
> > > >     if (rpmb_route_frames(rdev, p0, params[0].u.memref.size, p1,
> > > >                           params[1].u.memref.size)) {
> > > >             arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > …
> > >
> > > How do you think about to use an additional label for the shown
> > > error code assignment?
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v7.0-rc2#n526
> > >
> >
> > I suppose here you meant to update the label name since it's the same
> > error type used by other code paths too. So following label rename
> > should be fine I think as per coding guidelines:
> >
> > s/out/err_dev_put/
> 
> Wouldn't the name err_dev_put suggest this only occurs in the error path?
> 

Okay, let rather rename it to out_dev_put.

-Sumit