Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions drivers/tee/optee/ffa_abi.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,28 @@ static int optee_ffa_from_msg_param(struct optee *optee,
return 0;
}

/*
* OP-TEE FF-A memory objects use 4 KiB pages while the kernel page size may
* be larger, for example 64 KiB on arm64. When the shared memory offset from
* the start of its first kernel page exceeds 4 KiB, pass it in offs_low and
* offs_high instead of internal_offs.
*/
static void optee_ffa_set_fmem_offsets(struct optee_msg_param_fmem *fmem,
struct tee_shm *shm, u64 shm_offs)
{
unsigned long page_offs = tee_shm_get_page_offset(shm);

if (page_offs < FFA_PAGE_SIZE) {
fmem->internal_offs = page_offs;
} else {
fmem->internal_offs = 0;
shm_offs += page_offs;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can moving the page prefix into the normal offset fields break round trips? For a page prefix P and logical offset O, this sends P+O, while from_msg_param_ffa_mem() copies that value directly back into the shm-relative tee_param.memref.shm_offs. Because optee_invoke_func() overwrites the caller's parameter, a kernel client that reuses it sends 2P+O next time.

No current in-tree kernel client reuses that value: STMM's buffer is page-aligned, while trusted-key buffers may be unaligned but their parameters are rebuilt for each invocation and only the returned size is consumed. This is a latent serialization-contract issue, not a demonstrated in-tree access.

Could optee_ffa_shm_register() instead start the FF-A descriptor at Q = round_down(P, FFA_PAGE_SIZE), reduce its length by Q, and retain only P-Q in internal_offs? The normal offset would then remain O in both directions.

}

fmem->offs_low = shm_offs;
fmem->offs_high = shm_offs >> 32;
}

static int to_msg_param_ffa_mem(struct optee_msg_param *mp,
const struct tee_param *p)
{
Expand All @@ -196,14 +218,15 @@ static int to_msg_param_ffa_mem(struct optee_msg_param *mp,
TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT;

if (shm) {
u64 shm_offs = p->u.memref.shm_offs;
u64 total_offs = p->u.memref.shm_offs;

mp->u.fmem.internal_offs = shm->offset;
if (tee_shm_get_page_offset(shm) >= FFA_PAGE_SIZE)
total_offs += tee_shm_get_page_offset(shm);

mp->u.fmem.offs_low = shm_offs;
mp->u.fmem.offs_high = shm_offs >> 32;
optee_ffa_set_fmem_offsets(&mp->u.fmem, shm,
p->u.memref.shm_offs);
/* Check that the entire offset could be stored. */
if (mp->u.fmem.offs_high != shm_offs >> 32)
if (mp->u.fmem.offs_high != total_offs >> 32)
return -EINVAL;

mp->u.fmem.global_id = shm->sec_world_id;
Expand Down Expand Up @@ -458,8 +481,8 @@ static void handle_ffa_rpc_func_cmd_shm_alloc(struct tee_context *ctx,
.attr = OPTEE_MSG_ATTR_TYPE_FMEM_OUTPUT,
.u.fmem.size = tee_shm_get_size(shm),
.u.fmem.global_id = shm->sec_world_id,
.u.fmem.internal_offs = shm->offset,
};
optee_ffa_set_fmem_offsets(&arg->params[0].u.fmem, shm, 0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What consumes offs_low and offs_high in this reply? OP-TEE's thread_rpc_alloc() reads only internal_offs and global_id before calling mobj_ffa_get_by_cookie(). For P >= 4 KiB, this therefore returns internal_offs=0 and hides P in fields that are ignored, so secure world maps the FF-A object base instead of the supplicant buffer at P.

The reference supplicant is page-aligned, but the Linux supplicant UAPI permits arbitrary alignment, while OP-TEE requests only 8-byte alignment. Before this change P was rejected as an oversized internal_offs; afterward it can succeed with a wrong-base mapping. Would the descriptor-base split above avoid changing this reply's established internal_offs semantics?


arg->ret = TEEC_SUCCESS;
}
Expand Down
Loading