[PATCH] KVM: arm64: Validate the FF-A memory access descriptor placement

[PATCH] KVM: arm64: Validate the FF-A memory access descriptor placement

[Sebastian Ene]

Prevent the pKVM hypervisor from making assumptions that the
endpoint memory access descriptor (EMAD) comes right after the
FF-A memory region header and enforce a strict placement for it
when validating an FF-A memory lend/share transaction.

Prior to FF-A version 1.1 the header of the memory region
didn't contain an offset to the endpoint memory access descriptor.
The layout of a memory transaction looks like this:

  Field name				| Offset
					 -- 0
[ Header (ffa_mem_region)               |__ ep_mem_offset
  EMAD 1 (ffa_mem_region_attributes)	|
]

Reject the host from specifying a memory access descriptor offset
that is different than the size of the memory region header.

Cc: stable@vger.kernel.org
Fixes: 42fb33dde42b ("KVM: arm64: Use FF-A 1.1 with pKVM")
Signed-off-by: Sebastian Ene <sebastianene@google.com>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 94161ea1cd60..0703c0ad8dff 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -508,6 +508,12 @@ static void __do_ffa_mem_xfer(const u64 func_id,
 	buf = hyp_buffers.tx;
 	memcpy(buf, host_buffers.tx, fraglen);
 
+	if (FFA_MEM_REGION_HAS_EP_MEM_OFFSET(hyp_ffa_version) &&
+	    buf->ep_mem_offset != sizeof(struct ffa_mem_region)) {
+		ret = FFA_RET_INVALID_PARAMETERS;
+		goto out_unlock;
+	}
+
 	ep_mem_access = (void *)buf +
 			ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
 	offset = ep_mem_access->composite_off;
-- 
2.54.0.rc1.555.g9c883467ad-goog

Re: [PATCH] KVM: arm64: Validate the FF-A memory access descriptor placement

[Marc Zyngier]

On Wed, 22 Apr 2026 11:25:40 +0100,
Sebastian Ene <sebastianene@google.com> wrote:
> 
> Prevent the pKVM hypervisor from making assumptions that the
> endpoint memory access descriptor (EMAD) comes right after the
> FF-A memory region header and enforce a strict placement for it
> when validating an FF-A memory lend/share transaction.

As I read this, you want to remove a bad assumption...

> 
> Prior to FF-A version 1.1 the header of the memory region
> didn't contain an offset to the endpoint memory access descriptor.
> The layout of a memory transaction looks like this:
> 
>   Field name				| Offset
> 					 -- 0
> [ Header (ffa_mem_region)               |__ ep_mem_offset
>   EMAD 1 (ffa_mem_region_attributes)	|
> ]
> 
> Reject the host from specifying a memory access descriptor offset
> that is different than the size of the memory region header.

And yet you decide that you want to enforce this assumption. I don't
understand how you arrive to this conclusion.

Looking at the spec, it appears that the offset is *designed* to allow
a gap between the header and the EMAD. Refusing to handle a it seems to be a
violation of the spec.

What am I missing?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH] KVM: arm64: Validate the FF-A memory access descriptor placement

[Sebastian Ene]

On Wed, Apr 22, 2026 at 01:24:02PM +0100, Marc Zyngier wrote:
> On Wed, 22 Apr 2026 11:25:40 +0100,
> Sebastian Ene <sebastianene@google.com> wrote:
> > 
> > Prevent the pKVM hypervisor from making assumptions that the
> > endpoint memory access descriptor (EMAD) comes right after the
> > FF-A memory region header and enforce a strict placement for it
> > when validating an FF-A memory lend/share transaction.

Hello Marc,

> 
> As I read this, you want to remove a bad assumption...
> 
> > 
> > Prior to FF-A version 1.1 the header of the memory region
> > didn't contain an offset to the endpoint memory access descriptor.
> > The layout of a memory transaction looks like this:
> > 
> >   Field name				| Offset
> > 					 -- 0
> > [ Header (ffa_mem_region)               |__ ep_mem_offset
> >   EMAD 1 (ffa_mem_region_attributes)	|
> > ]
> > 
> > Reject the host from specifying a memory access descriptor offset
> > that is different than the size of the memory region header.
> 
> And yet you decide that you want to enforce this assumption. I don't
> understand how you arrive to this conclusion.
> 
> Looking at the spec, it appears that the offset is *designed* to allow
> a gap between the header and the EMAD. Refusing to handle a it seems to be a
> violation of the spec.
> 
> What am I missing?

While the spec allows the gap to be variable (since version 1.1), the
arm ff-a driver places it at a fixed position in:
ffa_mem_region_additional_setup() 
https://elixir.bootlin.com/linux/v7.0/source/drivers/firmware/arm_ffa/driver.c#L671

and makes use of the same assumption in: ffa_mem_desc_offset().
https://elixir.bootlin.com/linux/v7.0/source/include/linux/arm_ffa.h#L448
The later one seems wrong IMO. because we should compute the offset
based on the value stored in ep_mem_offset and not adding it up with
sizeof(struct ffa_mem_region).

Maybe this should be the fix instead and not the one in pKVM ? What do
you think ?

The current implementation in pKVM makes use of the
ffa_mem_desc_offset() to validate the first EMAD. If a compromised host
places an EMAD at a different offset than sizeof(struct ffa_mem_region),
then pKVM will not validate that EMAD.

> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

Thanks,
Sebastian

Re: [PATCH] KVM: arm64: Validate the FF-A memory access descriptor placement

[Sudeep Holla]

On Wed, Apr 22, 2026 at 01:35:55PM +0000, Sebastian Ene wrote:
> On Wed, Apr 22, 2026 at 01:24:02PM +0100, Marc Zyngier wrote:
> > On Wed, 22 Apr 2026 11:25:40 +0100,
> > Sebastian Ene <sebastianene@google.com> wrote:
> > > 
> > > Prevent the pKVM hypervisor from making assumptions that the
> > > endpoint memory access descriptor (EMAD) comes right after the
> > > FF-A memory region header and enforce a strict placement for it
> > > when validating an FF-A memory lend/share transaction.
> 
> Hello Marc,
> 
> > 
> > As I read this, you want to remove a bad assumption...
> > 
> > > 
> > > Prior to FF-A version 1.1 the header of the memory region
> > > didn't contain an offset to the endpoint memory access descriptor.
> > > The layout of a memory transaction looks like this:
> > > 
> > >   Field name				| Offset
> > > 					 -- 0
> > > [ Header (ffa_mem_region)               |__ ep_mem_offset
> > >   EMAD 1 (ffa_mem_region_attributes)	|
> > > ]
> > > 
> > > Reject the host from specifying a memory access descriptor offset
> > > that is different than the size of the memory region header.
> > 
> > And yet you decide that you want to enforce this assumption. I don't
> > understand how you arrive to this conclusion.
> > 
> > Looking at the spec, it appears that the offset is *designed* to allow
> > a gap between the header and the EMAD. Refusing to handle a it seems to be a
> > violation of the spec.
> > 
> > What am I missing?
> 
> While the spec allows the gap to be variable (since version 1.1), the
> arm ff-a driver places it at a fixed position in:
> ffa_mem_region_additional_setup() 
> https://elixir.bootlin.com/linux/v7.0/source/drivers/firmware/arm_ffa/driver.c#L671
> 

That's just the current choice in the driver and can be changed in the future.

> and makes use of the same assumption in: ffa_mem_desc_offset().
> https://elixir.bootlin.com/linux/v7.0/source/include/linux/arm_ffa.h#L448

Again this is just in the transmit path of the message the driver is
constructing and hence it is a simple choice rather than wrong assumption.

> The later one seems wrong IMO. because we should compute the offset
> based on the value stored in ep_mem_offset and not adding it up with
> sizeof(struct ffa_mem_region).
> 

Sorry what am I missing as the driver is building these descriptors to
send it across to SPMC, we are populating the field and it will be 0
before it is initialised

> Maybe this should be the fix instead and not the one in pKVM ? What do
> you think ?
> 

Can you share the diff you have in mind to understand your concern better
or are you referring to this patch itself.

> The current implementation in pKVM makes use of the
> ffa_mem_desc_offset() to validate the first EMAD. If a compromised host
> places an EMAD at a different offset than sizeof(struct ffa_mem_region),
> then pKVM will not validate that EMAD.
>

Calling the host as compromised if it chooses a different offset seems bit
of extreme here. I am no sure if I am missing to understand something here.

-- 
Regards,
Sudeep

Re: [PATCH] KVM: arm64: Validate the FF-A memory access descriptor placement

[Sudeep Holla]

On Wed, Apr 22, 2026 at 01:24:02PM +0100, Marc Zyngier wrote:
> On Wed, 22 Apr 2026 11:25:40 +0100,
> Sebastian Ene <sebastianene@google.com> wrote:
> > 
> > Prevent the pKVM hypervisor from making assumptions that the
> > endpoint memory access descriptor (EMAD) comes right after the
> > FF-A memory region header and enforce a strict placement for it
> > when validating an FF-A memory lend/share transaction.
> 
> As I read this, you want to remove a bad assumption...
> 

Indeed, it matches my understanding as well. I got confused with the
code change initially only to realise you want to restrict the choice
of offset.

> > 
> > Prior to FF-A version 1.1 the header of the memory region
> > didn't contain an offset to the endpoint memory access descriptor.
> > The layout of a memory transaction looks like this:
> > 
> >   Field name				| Offset
> > 					 -- 0
> > [ Header (ffa_mem_region)               |__ ep_mem_offset
> >   EMAD 1 (ffa_mem_region_attributes)	|
> > ]
> > 
> > Reject the host from specifying a memory access descriptor offset
> > that is different than the size of the memory region header.
> 
> And yet you decide that you want to enforce this assumption. I don't
> understand how you arrive to this conclusion.
> 
> Looking at the spec, it appears that the offset is *designed* to allow
> a gap between the header and the EMAD. Refusing to handle a it seems to be a
> violation of the spec.
> 

+1

-- 
Regards,
Sudeep