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,
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
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
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
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
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
op-tee@lists.trustedfirmware.org
-
Amirreza Zarrabi -
Robertus Diawan Chris