* [PATCH] optee: Check return value of tee_shm_get_va()
@ 2026-03-05 8:33 Chen Ni
2026-03-06 11:52 ` Markus Elfring via OP-TEE
0 siblings, 1 reply; 5+ messages in thread
From: Chen Ni @ 2026-03-05 8:33 UTC (permalink / raw)
To: jens.wiklander; +Cc: sumit.garg, ulf.hansson, op-tee, linux-kernel, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] optee: Check return value of tee_shm_get_va()
2026-03-05 8:33 [PATCH] optee: Check return value of tee_shm_get_va() Chen Ni
@ 2026-03-06 11:52 ` Markus Elfring via OP-TEE
2026-03-12 9:16 ` Sumit Garg via OP-TEE
0 siblings, 1 reply; 5+ messages in thread
From: Markus Elfring via OP-TEE @ 2026-03-06 11:52 UTC (permalink / raw)
To: Chen Ni, op-tee, Jens Wiklander; +Cc: LKML, Sumit Garg, Ulf Hansson
…
> +++ 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] optee: Check return value of tee_shm_get_va()
2026-03-06 11:52 ` Markus Elfring via OP-TEE
@ 2026-03-12 9:16 ` Sumit Garg via OP-TEE
2026-03-12 11:16 ` Jens Wiklander
0 siblings, 1 reply; 5+ messages in thread
From: Sumit Garg via OP-TEE @ 2026-03-12 9:16 UTC (permalink / raw)
To: Markus Elfring; +Cc: Chen Ni, op-tee, LKML, Ulf Hansson
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] optee: Check return value of tee_shm_get_va()
2026-03-12 9:16 ` Sumit Garg via OP-TEE
@ 2026-03-12 11:16 ` Jens Wiklander
2026-03-15 5:02 ` Sumit Garg via OP-TEE
0 siblings, 1 reply; 5+ messages in thread
From: Jens Wiklander @ 2026-03-12 11:16 UTC (permalink / raw)
To: Sumit Garg; +Cc: Markus Elfring, Chen Ni, op-tee, LKML, Ulf Hansson
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] optee: Check return value of tee_shm_get_va()
2026-03-12 11:16 ` Jens Wiklander
@ 2026-03-15 5:02 ` Sumit Garg via OP-TEE
0 siblings, 0 replies; 5+ messages in thread
From: Sumit Garg via OP-TEE @ 2026-03-15 5:02 UTC (permalink / raw)
To: Jens Wiklander; +Cc: Markus Elfring, Chen Ni, op-tee, LKML, Ulf Hansson
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-15 5:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-05 8:33 [PATCH] optee: Check return value of tee_shm_get_va() Chen Ni
2026-03-06 11:52 ` Markus Elfring via OP-TEE
2026-03-12 9:16 ` Sumit Garg via OP-TEE
2026-03-12 11:16 ` Jens Wiklander
2026-03-15 5:02 ` Sumit Garg via OP-TEE
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox