[PATCH] tee: fix uninitialized pointers with free attr

Ally Heev posted 1 patch 1 month, 1 week ago
drivers/tee/qcomtee/call.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] tee: fix uninitialized pointers with free attr
Posted by Ally Heev 1 month, 1 week ago
Uninitialized pointers with `__free` attribute can cause undefined
behaviour as the memory assigned(randomly) to the pointer is freed
automatically when the pointer goes out of scope

tee doesn't have any bugs related to this as of now, but
it is better to initialize and assign pointers with `__free` attr
in one statement to ensure proper scope-based cleanup

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
Signed-off-by: Ally Heev <allyheev@gmail.com>
---
 drivers/tee/qcomtee/call.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tee/qcomtee/call.c b/drivers/tee/qcomtee/call.c
index ac134452cc9cfd384c28d41547545f2c5748d86c..8b7b4decddd8d1811dc0a7cc46a4a4fbada45526 100644
--- a/drivers/tee/qcomtee/call.c
+++ b/drivers/tee/qcomtee/call.c
@@ -645,12 +645,13 @@ static void qcomtee_get_version(struct tee_device *teedev,
 static void qcomtee_get_qtee_feature_list(struct tee_context *ctx, u32 id,
 					  u32 *version)
 {
-	struct qcomtee_object_invoke_ctx *oic __free(kfree);
 	struct qcomtee_object *client_env, *service;
 	struct qcomtee_arg u[3] = { 0 };
 	int result;
 
-	oic = qcomtee_object_invoke_ctx_alloc(ctx);
+	struct qcomtee_object_invoke_ctx *oic __free(kfree) =
+		qcomtee_object_invoke_ctx_alloc(ctx);
+
 	if (!oic)
 		return;
 

---
base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
change-id: 20251105-aheev-uninitialized-free-attr-tee-0221e45ec5a2

Best regards,
-- 
Ally Heev <allyheev@gmail.com>
Re: [PATCH] tee: fix uninitialized pointers with free attr
Posted by Jens Wiklander 1 month, 1 week ago
Hi,

On Wed, Nov 5, 2025 at 3:20 PM Ally Heev <allyheev@gmail.com> wrote:
>
> Uninitialized pointers with `__free` attribute can cause undefined
> behaviour as the memory assigned(randomly) to the pointer is freed
> automatically when the pointer goes out of scope
>
> tee doesn't have any bugs related to this as of now, but
> it is better to initialize and assign pointers with `__free` attr
> in one statement to ensure proper scope-based cleanup
>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
> Signed-off-by: Ally Heev <allyheev@gmail.com>
> ---
>  drivers/tee/qcomtee/call.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tee/qcomtee/call.c b/drivers/tee/qcomtee/call.c
> index ac134452cc9cfd384c28d41547545f2c5748d86c..8b7b4decddd8d1811dc0a7cc46a4a4fbada45526 100644
> --- a/drivers/tee/qcomtee/call.c
> +++ b/drivers/tee/qcomtee/call.c
> @@ -645,12 +645,13 @@ static void qcomtee_get_version(struct tee_device *teedev,
>  static void qcomtee_get_qtee_feature_list(struct tee_context *ctx, u32 id,
>                                           u32 *version)
>  {
> -       struct qcomtee_object_invoke_ctx *oic __free(kfree);
>         struct qcomtee_object *client_env, *service;
>         struct qcomtee_arg u[3] = { 0 };
>         int result;
>
> -       oic = qcomtee_object_invoke_ctx_alloc(ctx);
> +       struct qcomtee_object_invoke_ctx *oic __free(kfree) =
> +               qcomtee_object_invoke_ctx_alloc(ctx);

Can we zero-initialize oic at its original location instead? Doing it
here looks messy.

Thanks,
Jens

> +
>         if (!oic)
>                 return;
>
>
> ---
> base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
> change-id: 20251105-aheev-uninitialized-free-attr-tee-0221e45ec5a2
>
> Best regards,
> --
> Ally Heev <allyheev@gmail.com>
>
Re: [PATCH] tee: fix uninitialized pointers with free attr
Posted by ally heev 1 month, 1 week ago
On Mon, 2025-11-10 at 09:57 +0100, Jens Wiklander wrote:
[..]
> > diff --git a/drivers/tee/qcomtee/call.c b/drivers/tee/qcomtee/call.c
> > index ac134452cc9cfd384c28d41547545f2c5748d86c..8b7b4decddd8d1811dc0a7cc46a4a4fbada45526 100644
> > --- a/drivers/tee/qcomtee/call.c
> > +++ b/drivers/tee/qcomtee/call.c
> > @@ -645,12 +645,13 @@ static void qcomtee_get_version(struct tee_device *teedev,
> >  static void qcomtee_get_qtee_feature_list(struct tee_context *ctx, u32 id,
> >                                           u32 *version)
> >  {
> > -       struct qcomtee_object_invoke_ctx *oic __free(kfree);
> >         struct qcomtee_object *client_env, *service;
> >         struct qcomtee_arg u[3] = { 0 };
> >         int result;
> > 
> > -       oic = qcomtee_object_invoke_ctx_alloc(ctx);
> > +       struct qcomtee_object_invoke_ctx *oic __free(kfree) =
> > +               qcomtee_object_invoke_ctx_alloc(ctx);
> 
> Can we zero-initialize oic at its original location instead? Doing it
> here looks messy.
> 
> Thanks,
> Jens
> 
> > +
> >         if (!oic)
> >                 return;
> > 
> > 
> > ---
> > base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
> > change-id: 20251105-aheev-uninitialized-free-attr-tee-0221e45ec5a2
> > 
> > Best regards,
> > --
> > Ally Heev <allyheev@gmail.com>
> > 

Thanks, will fix it

Regards,
ALly