All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Bertrand Marquis <Bertrand.Marquis@arm.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Volodymyr Babchuk <volodymyr_babchuk@epam.com>,
	Jens Wiklander <jenswi@kernel.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>,
	Michal Orzel <michal.orzel@amd.com>
Subject: Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
Date: Tue, 18 Aug 2026 15:46:41 +0100	[thread overview]
Message-ID: <07151e7f-3179-4432-b7fe-e70890f1ba4f@citrix.com> (raw)
In-Reply-To: <7F03E2BE-44A0-41E9-850C-8480CA0C8F0D@arm.com>

On 18/08/2026 2:28 pm, Bertrand Marquis wrote:
> Hi Andrew,
>
>> On 18 Aug 2026, at 14:40, Andrew Cooper <andrew.cooper3@citrix.com> 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@arm.com>
>>> ---
>>> 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


  reply	other threads:[~2026-08-18 14:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-18 12:16 [PATCH] xen/arm: ffa: Harden SEND2 against invented loads Bertrand Marquis
2026-08-18 12:40 ` Andrew Cooper
2026-08-18 13:28   ` Bertrand Marquis
2026-08-18 14:46     ` Andrew Cooper [this message]
2026-08-19  6:39       ` Bertrand Marquis
2026-08-19 10:21         ` Andrew Cooper
2026-08-19  7:47 ` Orzel, Michal
2026-08-19  8:01   ` Bertrand Marquis
2026-08-19  8:09     ` Orzel, Michal
2026-08-19  8:18       ` Bertrand Marquis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=07151e7f-3179-4432-b7fe-e70890f1ba4f@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Bertrand.Marquis@arm.com \
    --cc=jenswi@kernel.org \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=sstabellini@kernel.org \
    --cc=volodymyr_babchuk@epam.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.