Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/08/2026 7:39 am, Bertrand Marquis wrote:
> Hi Andrew,
>
>> On 18 Aug 2026, at 16:46, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>>
>> On 18/08/2026 2:28 pm, Bertrand Marquis wrote:
>>> Hi Andrew,
>>>
>>>> On 18 Aug 2026, at 14:40, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 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 <bertrand.marquis@xxxxxxx>
>>>>> ---
>>>>> 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.
> No worries. 
> Would you consider renaming them a requirement for this patch? 
> If not, I would prefer to keep this fix focused on adding the barrier and 
> avoid unrelated churn.

This would be later cleanup.  It definitely shouldn't be part of this fix.

~Andrew