Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
Andrew Cooper <[email protected]>
| Newsgroups | gmane.comp.emulators.xen.devel |
|---|---|
| Message-ID | <[email protected]> |
On 18/08/2026 2:28 pm, Bertrand Marquis wrote: > Hi Andrew, > >> On 18 Aug 2026, at 14:40, Andrew Cooper <[email protected]> wrote: >> >> On 18/08/2026 1:16 pm, Bertrand Marquis wrote: >>> Research into compiler-invented loads has flagged FFA_MSG_SEND2 as a >>> possible vulnerability. >>> >>> ffa_handle_msg_send2() copies the message header from the guest-writable >>> TX buffer before validating and using its fields. A plain structure copy >>> does not prevent the compiler from re-deriving later field accesses from >>> the live TX mapping. >>> >>> For VM-to-VM messages, msg_offset and msg_size are validated against the >>> source and destination buffers, then used to copy the payload. If a >>> sibling vCPU changes the header and the compiler reloads either field, >>> the checked and used values can differ. This can cause an out-of-bounds >>> read from the sender's TX buffer or an out-of-bounds write into the >>> receiver's RX buffer. >>> >>> The cross-VM path is gated by CONFIG_FFA_VM_TO_VM, which is disabled by >>> default. The audit ranks the likelihood of such a reload as low, but the >>> C semantics do not guarantee that later accesses use the stack copy. >>> >>> Add a compiler barrier immediately after copying the header so that >>> validation and use consume the same snapshot. >>> >>> Link: https://github.com/xoreaxeaxeax/schrodingers-toctou/blob/main/observer-effect/audits/audit-xen-tee-mediator-RELEASE-4.21.1.md#tm-2--ff-a-txrx-buffers-ffa_shmc-ffa_msgc >>> Fixes: 98af565b1e61 ("xen/arm: ffa: Add indirect message between VM") >>> Signed-off-by: Bertrand Marquis <[email protected]> >>> --- >>> xen/arch/arm/tee/ffa_msg.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/xen/arch/arm/tee/ffa_msg.c b/xen/arch/arm/tee/ffa_msg.c >>> index 1eadc62870f2..39f561c8237f 100644 >>> --- a/xen/arch/arm/tee/ffa_msg.c >>> +++ b/xen/arch/arm/tee/ffa_msg.c >>> @@ -257,6 +257,11 @@ int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs) >>> >>> /* create a copy of the message header */ >>> memcpy(&src_msg, tx_buf, sizeof(src_msg)); >>> + /* >>> + * Make sure that "tx_buf" which is shared with the guest isn't accessed >>> + * again after this point. >>> + */ >>> + barrier(); >>> >>> src_id = src_msg.send_recv_id >> 16; >>> dst_id = src_msg.send_recv_id & GENMASK(15,0); >> This does look to be adequate to fix the potential issue, but you should >> drop the ACCESS_ONCE(src_ctx->guest_vers) a little lower down. >> >> With a safe copy on the stack, there's no need to further inhibit >> optimisations around it. In fact, it's unclear why e040b94d0fff added >> the ACCESS_ONCE() in the first place, seeing as it was already an >> on-stack object at the time. > the ACCESS_ONCE is protecting the access to guest_vers which is not on the stack > but a value on an internal context accessed by all VMs. > > You probably mixed src_MSG with src_CTX ? Oh, maybe. Those really ought to have more distinct names. ~Andrew