tee: optee: Fix FF-A offset handling on 64 KiB pages - #553
tee: optee: Fix FF-A offset handling on 64 KiB pages#553mahantesh-nvidia wants to merge 1 commit into
Conversation
FF-A memory objects use 4 KiB pages, but the kernel page size may be larger. On systems with 64 KiB pages, offsets beyond the first 4 KiB must be encoded in offs_low and offs_high instead of internal_offs. Fixes: 4615e5a ("optee: add FF-A support") Signed-off-by: Mahantesh Salimath <mahantesh@nvidia.com>
|
This patch is needed for Bluefield-4 product/project. This fix is part of an effort to integrate OP-TEE OS as a Secure Partition running under Hafnium (SEL2). Link to Feature Request: (Redmine) https://redmine.mellanox.com/issues/4654738 Due to project deadlines and customer requests prioritizing the submission here and will soon submit the patch for upstream as well. |
BaseOS Kernel ReviewWarning
|
PR Validation ReportPatchscan ✅ No Missing FixesAll cherry-picked commits checked — no missing upstream fixes found. PR Lint ❌ Errors foundDetailsChecking 1 commits...
Cherry-pick digest:
┌──────────────┬──────────────────────────────────────────────────────────────────┬────────────┬─────────┬───────────────────────────┐
│ Local │ Referenced upstream / Patch subject │ Patch-ID │ Subject │ SoB chain │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ b62fa27d8b77 │ [SAUCE] tee: optee: fix ff-a offset handling on 64 kib pages │ N/A │ N/A │ mahantes │
└──────────────┴──────────────────────────────────────────────────────────────────┴────────────┴─────────┴───────────────────────────┘
Lint results:
E: b62fa27d8b77 ("tee: optee: Fix FF-A offset handling on 64 KiB pag"): not SAUCE/UBUNTU/Revert but has no upstream reference trailer (cherry picked from commit ... or backported from ...)
PR metadata:
W: PR title missing [<branch>] prefix: "tee: optee: Fix FF-A offset handling on 64 KiB pages"
E: PR targets 26.04_linux-nvidia-bos but body has no https://bugs.launchpad.net/... link
|
|
My comments from here still apply to this PR: #552 (comment) |
Ack, I will submit this patch upstream and include the LKML link as recommended. |
|
like matt said the PR needs the tag: "NVIDIA: VR: SAUCE:" |
jamieNguyenNVIDIA
left a comment
jamieNguyenNVIDIA
left a comment
There was a problem hiding this comment.
Two concerns with moving the page prefix into the normal FF-A offset fields.
| fmem->internal_offs = page_offs; | ||
| } else { | ||
| fmem->internal_offs = 0; | ||
| shm_offs += page_offs; |
There was a problem hiding this comment.
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.
| .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); |
There was a problem hiding this comment.
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?
5 participants
FF-A memory objects use 4 KiB pages, but the kernel page size may be larger. On systems with 64 KiB pages, offsets beyond the first 4 KiB must be encoded in offs_low and offs_high instead of internal_offs.
Fixes: 4615e5a ("optee: add FF-A support")