* [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
@ 2026-08-18 12:16 Bertrand Marquis
2026-08-18 12:40 ` Andrew Cooper
2026-08-19 7:47 ` Orzel, Michal
0 siblings, 2 replies; 10+ messages in thread
From: Bertrand Marquis @ 2026-08-18 12:16 UTC (permalink / raw)
To: xen-devel
Cc: Volodymyr Babchuk, Jens Wiklander, Stefano Stabellini,
Julien Grall, Michal Orzel
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);
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
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-19 7:47 ` Orzel, Michal
1 sibling, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2026-08-18 12:40 UTC (permalink / raw)
To: Bertrand Marquis, xen-devel
Cc: Andrew Cooper, Volodymyr Babchuk, Jens Wiklander,
Stefano Stabellini, Julien Grall, Michal Orzel
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.
~Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
2026-08-18 12:40 ` Andrew Cooper
@ 2026-08-18 13:28 ` Bertrand Marquis
2026-08-18 14:46 ` Andrew Cooper
0 siblings, 1 reply; 10+ messages in thread
From: Bertrand Marquis @ 2026-08-18 13:28 UTC (permalink / raw)
To: Andrew Cooper
Cc: xen-devel@lists.xenproject.org, Volodymyr Babchuk, Jens Wiklander,
Stefano Stabellini, Julien Grall, Michal Orzel
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 ?
Cheers
Bertrand
>
> ~Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
2026-08-18 13:28 ` Bertrand Marquis
@ 2026-08-18 14:46 ` Andrew Cooper
2026-08-19 6:39 ` Bertrand Marquis
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2026-08-18 14:46 UTC (permalink / raw)
To: Bertrand Marquis
Cc: Andrew Cooper, xen-devel@lists.xenproject.org, Volodymyr Babchuk,
Jens Wiklander, Stefano Stabellini, Julien Grall, Michal Orzel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
2026-08-18 14:46 ` Andrew Cooper
@ 2026-08-19 6:39 ` Bertrand Marquis
2026-08-19 10:21 ` Andrew Cooper
0 siblings, 1 reply; 10+ messages in thread
From: Bertrand Marquis @ 2026-08-19 6:39 UTC (permalink / raw)
To: Andrew Cooper
Cc: xen-devel@lists.xenproject.org, Volodymyr Babchuk, Jens Wiklander,
Stefano Stabellini, Julien Grall, Michal Orzel
Hi Andrew,
> On 18 Aug 2026, at 16:46, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> 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.
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.
Cheers
Bertrand
>
> ~Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
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-19 7:47 ` Orzel, Michal
2026-08-19 8:01 ` Bertrand Marquis
1 sibling, 1 reply; 10+ messages in thread
From: Orzel, Michal @ 2026-08-19 7:47 UTC (permalink / raw)
To: Bertrand Marquis, xen-devel
Cc: Volodymyr Babchuk, Jens Wiklander, Stefano Stabellini,
Julien Grall
On 18-Aug-26 14:16, 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.
This is a bit misleading because it *is* accessed in ffa_msg_send2_vm. The
comment should say what you wrote as the last paragraph in the commit msg.
With that:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
~Michal
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
2026-08-19 7:47 ` Orzel, Michal
@ 2026-08-19 8:01 ` Bertrand Marquis
2026-08-19 8:09 ` Orzel, Michal
0 siblings, 1 reply; 10+ messages in thread
From: Bertrand Marquis @ 2026-08-19 8:01 UTC (permalink / raw)
To: Orzel, Michal
Cc: xen-devel@lists.xenproject.org, Volodymyr Babchuk, Jens Wiklander,
Stefano Stabellini, Julien Grall
Hi Michal,
> On 19 Aug 2026, at 09:47, Orzel, Michal <Michal.Orzel@amd.com> wrote:
>
>
>
> On 18-Aug-26 14:16, 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.
> This is a bit misleading because it *is* accessed in ffa_msg_send2_vm. The
> comment should say what you wrote as the last paragraph in the commit msg.
Yes, this should be something around:
Ensure validation and use of the message header use the same snapshot.
Do you agree ?
>
> With that:
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
>
Thanks.
Could that be fixed on commit or do you want me to send a v2 ?
Cheers
Bertrand
> ~Michal
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
2026-08-19 8:01 ` Bertrand Marquis
@ 2026-08-19 8:09 ` Orzel, Michal
2026-08-19 8:18 ` Bertrand Marquis
0 siblings, 1 reply; 10+ messages in thread
From: Orzel, Michal @ 2026-08-19 8:09 UTC (permalink / raw)
To: Bertrand Marquis
Cc: xen-devel@lists.xenproject.org, Volodymyr Babchuk, Jens Wiklander,
Stefano Stabellini, Julien Grall
On 19-Aug-26 10:01, Bertrand Marquis wrote:
> Hi Michal,
>
>> On 19 Aug 2026, at 09:47, Orzel, Michal <Michal.Orzel@amd.com> wrote:
>>
>>
>>
>> On 18-Aug-26 14:16, 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.
>> This is a bit misleading because it *is* accessed in ffa_msg_send2_vm. The
>> comment should say what you wrote as the last paragraph in the commit msg.
>
> Yes, this should be something around:
> Ensure validation and use of the message header use the same snapshot.
>
> Do you agree ?
Yes. I'll fix on commit.
~Michal
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
2026-08-19 8:09 ` Orzel, Michal
@ 2026-08-19 8:18 ` Bertrand Marquis
0 siblings, 0 replies; 10+ messages in thread
From: Bertrand Marquis @ 2026-08-19 8:18 UTC (permalink / raw)
To: Orzel, Michal
Cc: xen-devel@lists.xenproject.org, Volodymyr Babchuk, Jens Wiklander,
Stefano Stabellini, Julien Grall
> On 19 Aug 2026, at 10:09, Orzel, Michal <michal.orzel@amd.com> wrote:
>
>
>
> On 19-Aug-26 10:01, Bertrand Marquis wrote:
>> Hi Michal,
>>
>>> On 19 Aug 2026, at 09:47, Orzel, Michal <Michal.Orzel@amd.com> wrote:
>>>
>>>
>>>
>>> On 18-Aug-26 14:16, 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.
>>> This is a bit misleading because it *is* accessed in ffa_msg_send2_vm. The
>>> comment should say what you wrote as the last paragraph in the commit msg.
>>
>> Yes, this should be something around:
>> Ensure validation and use of the message header use the same snapshot.
>>
>> Do you agree ?
> Yes. I'll fix on commit.
Thanks :-)
Bertrand
>
> ~Michal
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen/arm: ffa: Harden SEND2 against invented loads
2026-08-19 6:39 ` Bertrand Marquis
@ 2026-08-19 10:21 ` Andrew Cooper
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Cooper @ 2026-08-19 10:21 UTC (permalink / raw)
To: Bertrand Marquis
Cc: Andrew Cooper, xen-devel@lists.xenproject.org, Volodymyr Babchuk,
Jens Wiklander, Stefano Stabellini, Julien Grall, Michal Orzel
On 19/08/2026 7:39 am, Bertrand Marquis wrote:
> Hi Andrew,
>
>> On 18 Aug 2026, at 16:46, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>> 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.
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-08-19 10:22 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
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.