From: Robertus Diawan Chris <robertusdchris@gmail.com>
To: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
Cc: sumit.garg@kernel.org, linux-arm-msm@vger.kernel.org,
op-tee@lists.trustedfirmware.org, linux-kernel@vger.kernel.org,
linux-kernel-mentees@lists.linuxfoundation.org,
skhan@linuxfoundation.org, me@brighamcampbell.com
Subject: Re: [PATCH] tee: qcomtee: add missing va_end in early return qcomtee_object_user_init()
Date: Tue, 19 May 2026 05:56:15 +0700 [thread overview]
Message-ID: <aguZD_vcJCwXY--l@soyboi> (raw)
In-Reply-To: <e626963b-7f20-43fd-b3e0-e16d9b6f13f8@oss.qualcomm.com>
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
prev parent reply other threads:[~2026-05-19 14:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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
2026-05-15 5:23 ` Robertus Diawan Chris
2026-05-18 6:36 ` Amirreza Zarrabi via OP-TEE
2026-05-18 22:56 ` Robertus Diawan Chris [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aguZD_vcJCwXY--l@soyboi \
--to=robertusdchris@gmail.com \
--cc=amirreza.zarrabi@oss.qualcomm.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=me@brighamcampbell.com \
--cc=op-tee@lists.trustedfirmware.org \
--cc=skhan@linuxfoundation.org \
--cc=sumit.garg@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox