Re: [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init()

[Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>]

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