Re: [PATCH RFC v2 6/8] tee: optee: execute yielding RPMI calls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] optee: unhandled unknown RPC result causes infinite loop in yielding call
- [High] optee: deadlock on RPMI_ERR_BUSY when no local threads are active
- [High] optee: uninitialized req.flags leaks stack memory to Secure World
- [Medium] optee: zero resume_token from suspended call causes infinite retry loop
--

commit e5243042ee3a40ae18df933976be9641326c5d08
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
Date:   Tue Oct 6 00:39:50 2026 +0000

    tee: optee: execute yielding RPMI calls
    
    OP-TEE commands can span multiple exchanges with normal world. A call
    may yield to request an RPC service or allow interrupt processing
    before continuing execution in secure world.
    
    Add the RPMI yielding-call path used by the common OP-TEE session
    operations. Submit the command and RPC argument buffers as ranges
    within a shared memory parcel.
    
    Handle RPC requests while the call is suspended and resume execution
    using the token returned by OP-TEE until the command completes.
    
    Use the common OP-TEE call queue to wait when an initial request is
    rejected with RPMI_ERR_BUSY, allowing another active call to complete
    before retrying.
    
    Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
> index 58d82678be98f..6e76316794c18 100644
> --- a/drivers/tee/optee/rpmi_abi.c
> +++ b/drivers/tee/optee/rpmi_abi.c

[ ... ]

> +/* Handle RPC command or interrupt returns from a yielding call. */
> +static void optee_rpmi_handle_rpc(struct tee_context *ctx, struct optee *optee,
> +				  u32 result, struct optee_msg_arg *arg)
> +{
> +	switch (result) {
> +	case OPTEE_RPMI_YIELDING_CALL_RETURN_RPC_CMD:
> +		optee_rpmi_handle_rpc_cmd(ctx, optee, arg);
> +		break;
> +	case OPTEE_RPMI_YIELDING_CALL_RETURN_INTERRUPT:
> +		break;
> +	default:
> +		pr_warn("Unknown RPC func 0x%x\n", result);
> +		break;
> +	}
> +}

[Severity: High]
Does this code risk entering an infinite loop? If the result is unknown,
optee_rpmi_handle_rpc() logs a warning but doesn't return an error code to
the caller.

The loop in optee_rpmi_yielding_call() below will then blindly update the
token and continue infinitely polling the TEE.

> +
> +/**
> + * optee_rpmi_yielding_call() - submit and resume a yielding RPMI command
> + * @ctx: calling context
> + * @req: initial command request
> + * @rpc_arg: shared RPC argument buffer
> + * @system_thread: caller requests TEE system thread support
> + *
> + * Only RPMI_ERR_BUSY rejection of the initial command permits retry.
> + *
> + * Return: zero on completion, or a negative error.
> + */
> +static int optee_rpmi_yielding_call(struct tee_context *ctx,
> +				    const struct optee_rpmi_call_req *req,
> +				    struct optee_msg_arg *rpc_arg,
> +				    bool system_thread)
> +{
> +	struct optee *optee = tee_get_drvdata(ctx->teedev);
> +	struct optee_rpmi_resume_req resume = {
> +		.op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_RESUME),
> +		/* resume_token is nonzero after OP-TEE suspends the call. */
> +		.resume_token = 0,
> +	};
> +	struct optee_rpmi_call_resp resp;
> +	struct optee_call_waiter waiter;
> +	u32 result;
> +	s32 status;
> +	int ret;
> +
> +	optee_cq_wait_init(&optee->call_queue, &waiter, system_thread);
> +	while (true) {
> +		if (resume.resume_token)
> +			ret = optee_rpmi_call_with_status(optee, &resume,
> +							  sizeof(resume), &resp,
> +							  sizeof(resp), &status);
> +		else
> +			ret = optee_rpmi_call_with_status(optee, req, sizeof(*req),
> +							  &resp, sizeof(resp),
> +							  &status);

[Severity: Medium]
Will this correctly handle a zero resume_token from a suspended call?

If the firmware suspends the call but incorrectly provides a resume_token
of 0, resume.resume_token will be 0 on the next iteration. This if
statement will evaluate to false, falling back to resending the initial
request instead of the resume command, which could cause an infinite retry
loop.

> +		if (ret)
> +			goto done;
> +
> +		switch (status) {
> +		case RPMI_SUCCESS:
> +			break;
> +		case RPMI_ERR_BUSY:
> +			if (!resume.resume_token) {
> +				optee_cq_wait_for_completion(&optee->call_queue,
> +							     &waiter);
> +				continue;
> +			}

[Severity: High]
Can this cause a deadlock if the BUSY state was triggered by an external
client?

If RPMI_ERR_BUSY is returned on the initial command, the task blocks
indefinitely in optee_cq_wait_for_completion() waiting for a local
completion. If the shared mailbox or service is busy processing requests
from another VM and the local kernel has no other active threads, no thread
will ever call optee_cq_wait_final() to wake up this task.

> +
> +			fallthrough;

[ ... ]

> +/* The caller supplies SHM with room for command and RPC args. */
> +static int optee_rpmi_do_call_with_arg(struct tee_context *ctx,
> +				       struct tee_shm *shm, u_int offs,
> +				       bool system_thread)
> +{
> +	struct optee *optee = tee_get_drvdata(ctx->teedev);
> +	struct optee_msg_arg *arg, *rpc_arg;
> +	struct optee_rpmi_call_req req;

[Severity: High]
Does this leak uninitialized kernel stack memory to the Secure World?

The struct optee_rpmi_call_req req is allocated on the stack here but is
not zero-initialized.

> +	size_t arg_size, rpc_size, rpc_offset;
> +	u32 parcel_id, nonce;
> +
> +	arg = tee_shm_get_va(shm, offs);
> +	if (IS_ERR(arg))
> +		return PTR_ERR(arg);
> +
> +	arg_size = OPTEE_MSG_GET_ARG_SIZE(arg->num_params);
> +	rpc_size = OPTEE_MSG_GET_ARG_SIZE(optee->rpc_param_count);
> +	rpc_offset = offs + arg_size;
> +	rpc_arg = tee_shm_get_va(shm, rpc_offset);
> +	if (IS_ERR(rpc_arg))
> +		return PTR_ERR(rpc_arg);
> +
> +	optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
> +
> +	req.op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_WITH_ARG);
> +	req.parcel_id = cpu_to_le32(parcel_id);
> +	req.nonce = cpu_to_le32(nonce);
> +	req.arg_offset = cpu_to_le64((u64)shm->offset + offs);
> +	req.rpc_offset = cpu_to_le64((u64)shm->offset + rpc_offset);
> +	req.arg_size = cpu_to_le32(arg_size);
> +	req.rpc_size = cpu_to_le32(rpc_size);

[Severity: High]
The code explicitly initializes most fields but misses req.flags. This
leaves req.flags containing uninitialized stack memory when req is passed
to optee_rpmi_yielding_call() and sent over the RPMI mailbox.

Could we initialize the whole struct to zero?

> +
> +	return optee_rpmi_yielding_call(ctx, &req, rpc_arg, system_thread);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-rpmi-tee-service-grp-dev-v2-0-72f222e23ec1@oss.qualcomm.com?part=6