All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init()
@ 2026-05-13  9:10 Robertus Diawan Chris
  2026-05-15  1:23   ` Amirreza Zarrabi via OP-TEE
  2026-05-15  1:31   ` Amirreza Zarrabi via OP-TEE
  0 siblings, 2 replies; 6+ messages in thread
From: Robertus Diawan Chris @ 2026-05-13  9:10 UTC (permalink / raw)
  To: amirreza.zarrabi, jens.wiklander, sumit.garg
  Cc: linux-arm-msm, op-tee, linux-kernel, linux-kernel-mentees, skhan,
	me

qcomtee_object_user_init() is a variadic function and when the function
return because there's no dispatch callback in QCOMTEE_OBJECT_TYPE_CB
case, there's no va_end to cleanup "ap" object initialized by va_start
and that can cause undefined behavior. So make sure to use va_end before
returning the error code when there's no dispatch callback.

This is reported by Coverity Scan as "Missing varargs init or cleanup".

Fixes: d6e290837e50 ("tee: add Qualcomm TEE driver")
Signed-off-by: Robertus Diawan Chris <robertusdchris@gmail.com>
---
I don't have the device, so I am not sure how to test this change.
Thank you.

 drivers/tee/qcomtee/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tee/qcomtee/core.c b/drivers/tee/qcomtee/core.c
index b1cb50e434f0..901a31e8201f 100644
--- a/drivers/tee/qcomtee/core.c
+++ b/drivers/tee/qcomtee/core.c
@@ -306,8 +306,10 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
 		break;
 	case QCOMTEE_OBJECT_TYPE_CB:
 		object->ops = ops;
-		if (!object->ops->dispatch)
-			return -EINVAL;
+		if (!object->ops->dispatch) {
+			ret = -EINVAL;
+			goto out;
+		}
 
 		/* If failed, "no-name". */
 		object->name = kvasprintf_const(GFP_KERNEL, fmt, ap);
@@ -320,6 +322,8 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
 	default:
 		ret = -EINVAL;
 	}
+
+out:
 	va_end(ap);
 
 	return ret;

base-commit: 5d6919055dec134de3c40167a490f33c74c12581
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init()
  2026-05-13  9:10 [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init() Robertus Diawan Chris
@ 2026-05-15  1:23   ` Amirreza Zarrabi via OP-TEE
  2026-05-15  1:31   ` Amirreza Zarrabi via OP-TEE
  1 sibling, 0 replies; 6+ messages in thread
From: Amirreza Zarrabi @ 2026-05-15  1:23 UTC (permalink / raw)
  To: Robertus Diawan Chris, jens.wiklander, sumit.garg
  Cc: linux-arm-msm, op-tee, linux-kernel, linux-kernel-mentees, skhan,
	me

Hi,

On 5/13/2026 7:10 PM, Robertus Diawan Chris wrote:
> qcomtee_object_user_init() is a variadic function and when the function
> return because there's no dispatch callback in QCOMTEE_OBJECT_TYPE_CB
> case, there's no va_end to cleanup "ap" object initialized by va_start
> and that can cause undefined behavior. So make sure to use va_end before
> returning the error code when there's no dispatch callback.
> 
> This is reported by Coverity Scan as "Missing varargs init or cleanup".
> 
> Fixes: d6e290837e50 ("tee: add Qualcomm TEE driver")
> Signed-off-by: Robertus Diawan Chris <robertusdchris@gmail.com>
> ---
> I don't have the device, so I am not sure how to test this change.
> Thank you.
> 
>  drivers/tee/qcomtee/core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tee/qcomtee/core.c b/drivers/tee/qcomtee/core.c
> index b1cb50e434f0..901a31e8201f 100644
> --- a/drivers/tee/qcomtee/core.c
> +++ b/drivers/tee/qcomtee/core.c
> @@ -306,8 +306,10 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
>  		break;
>  	case QCOMTEE_OBJECT_TYPE_CB:
>  		object->ops = ops;
> -		if (!object->ops->dispatch)
> -			return -EINVAL;
> +		if (!object->ops->dispatch) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
>  
>  		/* If failed, "no-name". */
>  		object->name = kvasprintf_const(GFP_KERNEL, fmt, ap);
> @@ -320,6 +322,8 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
>  	default:
>  		ret = -EINVAL;
>  	}
> +
> +out:
>  	va_end(ap);
>  
>  	return ret;
> 
> base-commit: 5d6919055dec134de3c40167a490f33c74c12581

Reviewed-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

Thanks.
Amir


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init()
@ 2026-05-15  1:23   ` Amirreza Zarrabi via OP-TEE
  0 siblings, 0 replies; 6+ messages in thread
From: Amirreza Zarrabi via OP-TEE @ 2026-05-15  1:23 UTC (permalink / raw)
  To: Robertus Diawan Chris, jens.wiklander, sumit.garg
  Cc: linux-arm-msm, op-tee, linux-kernel, linux-kernel-mentees, skhan,
	me

Hi,

On 5/13/2026 7:10 PM, Robertus Diawan Chris wrote:
> qcomtee_object_user_init() is a variadic function and when the function
> return because there's no dispatch callback in QCOMTEE_OBJECT_TYPE_CB
> case, there's no va_end to cleanup "ap" object initialized by va_start
> and that can cause undefined behavior. So make sure to use va_end before
> returning the error code when there's no dispatch callback.
> 
> This is reported by Coverity Scan as "Missing varargs init or cleanup".
> 
> Fixes: d6e290837e50 ("tee: add Qualcomm TEE driver")
> Signed-off-by: Robertus Diawan Chris <robertusdchris@gmail.com>
> ---
> I don't have the device, so I am not sure how to test this change.
> Thank you.
> 
>  drivers/tee/qcomtee/core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tee/qcomtee/core.c b/drivers/tee/qcomtee/core.c
> index b1cb50e434f0..901a31e8201f 100644
> --- a/drivers/tee/qcomtee/core.c
> +++ b/drivers/tee/qcomtee/core.c
> @@ -306,8 +306,10 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
>  		break;
>  	case QCOMTEE_OBJECT_TYPE_CB:
>  		object->ops = ops;
> -		if (!object->ops->dispatch)
> -			return -EINVAL;
> +		if (!object->ops->dispatch) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
>  
>  		/* If failed, "no-name". */
>  		object->name = kvasprintf_const(GFP_KERNEL, fmt, ap);
> @@ -320,6 +322,8 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
>  	default:
>  		ret = -EINVAL;
>  	}
> +
> +out:
>  	va_end(ap);
>  
>  	return ret;
> 
> base-commit: 5d6919055dec134de3c40167a490f33c74c12581

Reviewed-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

Thanks.
Amir


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init()
  2026-05-13  9:10 [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init() Robertus Diawan Chris
@ 2026-05-15  1:31   ` Amirreza Zarrabi via OP-TEE
  2026-05-15  1:31   ` Amirreza Zarrabi via OP-TEE
  1 sibling, 0 replies; 6+ messages in thread
From: Amirreza Zarrabi @ 2026-05-15  1:31 UTC (permalink / raw)
  To: Robertus Diawan Chris, jens.wiklander, sumit.garg
  Cc: linux-arm-msm, op-tee, linux-kernel, linux-kernel-mentees, skhan,
	me

Hi,

Forgot to mention: how about using a break instead of a goto.
Then feel free to add Reviewed-by.

Thanks,
Amir


On 5/13/2026 7:10 PM, Robertus Diawan Chris wrote:
> qcomtee_object_user_init() is a variadic function and when the function
> return because there's no dispatch callback in QCOMTEE_OBJECT_TYPE_CB
> case, there's no va_end to cleanup "ap" object initialized by va_start
> and that can cause undefined behavior. So make sure to use va_end before
> returning the error code when there's no dispatch callback.
> 
> This is reported by Coverity Scan as "Missing varargs init or cleanup".
> 
> Fixes: d6e290837e50 ("tee: add Qualcomm TEE driver")
> Signed-off-by: Robertus Diawan Chris <robertusdchris@gmail.com>
> ---
> I don't have the device, so I am not sure how to test this change.
> Thank you.
> 
>  drivers/tee/qcomtee/core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tee/qcomtee/core.c b/drivers/tee/qcomtee/core.c
> index b1cb50e434f0..901a31e8201f 100644
> --- a/drivers/tee/qcomtee/core.c
> +++ b/drivers/tee/qcomtee/core.c
> @@ -306,8 +306,10 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
>  		break;
>  	case QCOMTEE_OBJECT_TYPE_CB:
>  		object->ops = ops;
> -		if (!object->ops->dispatch)
> -			return -EINVAL;
> +		if (!object->ops->dispatch) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
>  
>  		/* If failed, "no-name". */
>  		object->name = kvasprintf_const(GFP_KERNEL, fmt, ap);
> @@ -320,6 +322,8 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
>  	default:
>  		ret = -EINVAL;
>  	}
> +
> +out:
>  	va_end(ap);
>  
>  	return ret;
> 
> base-commit: 5d6919055dec134de3c40167a490f33c74c12581


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init()
@ 2026-05-15  1:31   ` Amirreza Zarrabi via OP-TEE
  0 siblings, 0 replies; 6+ messages in thread
From: Amirreza Zarrabi via OP-TEE @ 2026-05-15  1:31 UTC (permalink / raw)
  To: Robertus Diawan Chris, jens.wiklander, sumit.garg
  Cc: linux-arm-msm, op-tee, linux-kernel, linux-kernel-mentees, skhan,
	me

Hi,

Forgot to mention: how about using a break instead of a goto.
Then feel free to add Reviewed-by.

Thanks,
Amir


On 5/13/2026 7:10 PM, Robertus Diawan Chris wrote:
> qcomtee_object_user_init() is a variadic function and when the function
> return because there's no dispatch callback in QCOMTEE_OBJECT_TYPE_CB
> case, there's no va_end to cleanup "ap" object initialized by va_start
> and that can cause undefined behavior. So make sure to use va_end before
> returning the error code when there's no dispatch callback.
> 
> This is reported by Coverity Scan as "Missing varargs init or cleanup".
> 
> Fixes: d6e290837e50 ("tee: add Qualcomm TEE driver")
> Signed-off-by: Robertus Diawan Chris <robertusdchris@gmail.com>
> ---
> I don't have the device, so I am not sure how to test this change.
> Thank you.
> 
>  drivers/tee/qcomtee/core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tee/qcomtee/core.c b/drivers/tee/qcomtee/core.c
> index b1cb50e434f0..901a31e8201f 100644
> --- a/drivers/tee/qcomtee/core.c
> +++ b/drivers/tee/qcomtee/core.c
> @@ -306,8 +306,10 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
>  		break;
>  	case QCOMTEE_OBJECT_TYPE_CB:
>  		object->ops = ops;
> -		if (!object->ops->dispatch)
> -			return -EINVAL;
> +		if (!object->ops->dispatch) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
>  
>  		/* If failed, "no-name". */
>  		object->name = kvasprintf_const(GFP_KERNEL, fmt, ap);
> @@ -320,6 +322,8 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
>  	default:
>  		ret = -EINVAL;
>  	}
> +
> +out:
>  	va_end(ap);
>  
>  	return ret;
> 
> base-commit: 5d6919055dec134de3c40167a490f33c74c12581


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init()
  2026-05-15  1:31   ` Amirreza Zarrabi via OP-TEE
  (?)
@ 2026-05-15  5:23   ` Robertus Diawan Chris
  -1 siblings, 0 replies; 6+ messages in thread
From: Robertus Diawan Chris @ 2026-05-15  5:23 UTC (permalink / raw)
  To: Amirreza Zarrabi
  Cc: jens.wiklander, sumit.garg, linux-arm-msm, op-tee, linux-kernel,
	linux-kernel-mentees, skhan, me

Hello Amir,

On Fri, May 15, 2026 at 11:31:50AM +1000, Amirreza Zarrabi wrote:
> On 5/13/2026 7:10 PM, Robertus Diawan Chris wrote:
> > qcomtee_object_user_init() is a variadic function and when the function
> > return because there's no dispatch callback in QCOMTEE_OBJECT_TYPE_CB
> > case, there's no va_end to cleanup "ap" object initialized by va_start
> > and that can cause undefined behavior. So make sure to use va_end before
> > returning the error code when there's no dispatch callback.
> >
> > This is reported by Coverity Scan as "Missing varargs init or cleanup".
> >
> > Fixes: d6e290837e50 ("tee: add Qualcomm TEE driver")
> > Signed-off-by: Robertus Diawan Chris <robertusdchris@gmail.com>
> > ---
> > I don't have the device, so I am not sure how to test this change.
> > Thank you.
> >
> >  drivers/tee/qcomtee/core.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tee/qcomtee/core.c b/drivers/tee/qcomtee/core.c
> > index b1cb50e434f0..901a31e8201f 100644
> > --- a/drivers/tee/qcomtee/core.c
> > +++ b/drivers/tee/qcomtee/core.c
> > @@ -306,8 +306,10 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
> >  		break;
> >  	case QCOMTEE_OBJECT_TYPE_CB:
> >  		object->ops = ops;
> > -		if (!object->ops->dispatch)
> > -			return -EINVAL;
> > +		if (!object->ops->dispatch) {
> > +			ret = -EINVAL;
> > +			goto out;
> > +		}
> >
> >  		/* If failed, "no-name". */
> >  		object->name = kvasprintf_const(GFP_KERNEL, fmt, ap);
> > @@ -320,6 +322,8 @@ int qcomtee_object_user_init(struct qcomtee_object *object,
> >  	default:
> >  		ret = -EINVAL;
> >  	}
> > +
> > +out:
> >  	va_end(ap);
> >
> >  	return ret;
> >
> > base-commit: 5d6919055dec134de3c40167a490f33c74c12581
>
> Hi,
>
> Forgot to mention: how about using a break instead of a goto.

Oh right. In this case, using "break" statement is enough. I will send
the v2 of the patch. Maybe something like this:

	if (!object->ops->dispatch) {
		ret = -EINVAL;
		break;
	}

and then remove the "out" label.

> Then feel free to add Reviewed-by.

I want to confirm first, if I changed the patch using "break" statement,
do I need to add "Reviewed-by" tag in the v2 of the patch or not? I am
still not sure when to add "Reviewed-by" tag, like can we add
"Reviewed-by" tag when we changed the patch?

Thank you.

Best regards,
Robertus Diawan Chris

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-05-15  5:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13  9:10 [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init() Robertus Diawan Chris
2026-05-15  1:23 ` Amirreza Zarrabi
2026-05-15  1:23   ` Amirreza Zarrabi via OP-TEE
2026-05-15  1:31 ` Amirreza Zarrabi
2026-05-15  1:31   ` Amirreza Zarrabi via OP-TEE
2026-05-15  5:23   ` Robertus Diawan Chris

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.