* [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
2026-05-15 1:31 ` Amirreza Zarrabi
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 2026-05-15 1:31 ` Amirreza Zarrabi 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-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:31 ` Amirreza Zarrabi 2026-05-15 5:23 ` Robertus Diawan Chris 1 sibling, 1 reply; 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 @ 2026-05-15 5:23 ` Robertus Diawan Chris 2026-05-18 6:36 ` Amirreza Zarrabi 0 siblings, 1 reply; 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
* Re: [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init() 2026-05-15 5:23 ` Robertus Diawan Chris @ 2026-05-18 6:36 ` Amirreza Zarrabi 2026-05-18 22:56 ` Robertus Diawan Chris 0 siblings, 1 reply; 6+ messages in thread From: Amirreza Zarrabi @ 2026-05-18 6:36 UTC (permalink / raw) To: Robertus Diawan Chris Cc: jens.wiklander, sumit.garg, linux-arm-msm, op-tee, linux-kernel, linux-kernel-mentees, skhan, me Hi, On 5/15/2026 3:23 PM, Robertus Diawan Chris wrote: > 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? > This is a small change. You can add the tag when sending your v2 as long as you include the change. Best Regards, Amir > Thank you. > > Best regards, > Robertus Diawan Chris ^ 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-18 6:36 ` Amirreza Zarrabi @ 2026-05-18 22:56 ` Robertus Diawan Chris 0 siblings, 0 replies; 6+ messages in thread From: Robertus Diawan Chris @ 2026-05-18 22:56 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 Mon, May 18, 2026 at 04:36:20PM +1000, Amirreza Zarrabi wrote: > Hi, > > On 5/15/2026 3:23 PM, Robertus Diawan Chris wrote: > > 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? > > > > This is a small change. You can add the tag when sending your v2 > as long as you include the change. I see. Alright, I will send the patch v2 with the change and the "Reviewed-by" tag later. Thanks. Best regards, Robertus Diawan Chris ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-18 22:56 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:31 ` Amirreza Zarrabi 2026-05-15 5:23 ` Robertus Diawan Chris 2026-05-18 6:36 ` Amirreza Zarrabi 2026-05-18 22:56 ` Robertus Diawan Chris
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox