[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: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

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

[Marc Zyngier]

On Wed, 22 Apr 2026 14:35:55 +0100,
Sebastian Ene <sebastianene@google.com> 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 an implementation detail, and you shouldn't rely on this.

> 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 ?

I think you should parse the buffers as the spec intends them, without
assumptions or limitations.

> 
> 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.

Why compromised? Isn't that a perfectly valid thing to do? What I
understand is that the FFA 1.1 implementation in pKVM doesn't match
the expectations of the spec. If that's indeed the case, pKVM should
be fixed to accept these messages correctly, or stop using FFA 1.1.

	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 08:29:06PM +0100, Sudeep Holla wrote:
> 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
> > 
> 

Hello Sudeep,

> 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

Right, what I meant is having something like this since this function is not limited
to the driver scope and using it from other components would imply relying on the
assumption: 'ep_mem_offset == sizeof(struct ffa_mem_region)'. We will also have to validate
that the `ep_mem_offset` doesn't point outside of the mailbox designated buffer.

---
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index 81e603839c4a..62d67dae8b70 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -445,7 +445,7 @@ ffa_mem_desc_offset(struct ffa_mem_region *buf, int count, u32 ffa_version)
        if (!FFA_MEM_REGION_HAS_EP_MEM_OFFSET(ffa_version))
                offset += offsetof(struct ffa_mem_region, ep_mem_offset);
        else
-               offset += sizeof(struct ffa_mem_region);
+               offset += buf->ep_mem_offset;
 
        return offset;
 }
---

And then move `ffa_mem_region_additional_setup` to be called earlier before `ffa_mem_desc_offset`:
(so that it can setup the value for ep_mem_offset)

---
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index f2f94d4d533e..66de59c88aff 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -691,6 +691,8 @@ ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
        mem_region->flags = args->flags;
        mem_region->sender_id = drv_info->vm_id;
        mem_region->attributes = ffa_memory_attributes_get(func_id);
+
+       ffa_mem_region_additional_setup(drv_info->version, mem_region);
        composite_offset = ffa_mem_desc_offset(buffer, args->nattrs,
                                               drv_info->version);
 
@@ -708,7 +710,6 @@ ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
        }
        mem_region->handle = 0;
        mem_region->ep_count = args->nattrs;
-       ffa_mem_region_additional_setup(drv_info->version, mem_region);
---

> 
> > 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.

Sure, please let me know if you think this is wrong. I might have misunderstood it. 

> 
> > 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.
> 

Sorry for not explaining it, in pKVM model we don't trust the host kernel so we can assume that
everything that doesn't pass the hypervisor validation(in this case the ff-a memory transaction)
can be a potential attack that wants to compromise EL2.

> -- 
> Regards,
> Sudeep

Thanks,
Sebastian

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

[Sebastian Ene]

On Thu, Apr 23, 2026 at 09:08:46AM +0100, Marc Zyngier wrote:
> On Wed, 22 Apr 2026 14:35:55 +0100,
> Sebastian Ene <sebastianene@google.com> 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 an implementation detail, and you shouldn't rely on this.
> 
> > 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 ?
> 
> I think you should parse the buffers as the spec intends them, without
> assumptions or limitations.

Ack.

> 
> > 
> > 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.
> 
> Why compromised? Isn't that a perfectly valid thing to do? What I
> understand is that the FFA 1.1 implementation in pKVM doesn't match
> the expectations of the spec. If that's indeed the case, pKVM should
> be fixed to accept these messages correctly, or stop using FFA 1.1.
> 
> 	M.

Sorry, what I meant is that a potentially malicious host could abuse
this limitation of the FF-A proxy validation which is looking at a fixed
offset to do the EMAD validation. Another EMAD can be placed at a
different offset and it will bypass the validation of the proxy
alltogether.

We have two choices: the simple one is what this patch does (enforce a
fixed offset) or the second one : patch `ffa_mem_desc_offset` to use
ep_mem_offset instead of `sizeof(struct ffa_mem_region)` and validate 
the ep_mem_offset. 

> 
> -- 
> 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 Thu, Apr 23, 2026 at 09:17:49AM +0000, Sebastian Ene wrote:
> On Wed, Apr 22, 2026 at 08:29:06PM +0100, Sudeep Holla wrote:

[...]

> Hello Sudeep,
> 
> > 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
> 
> Right, what I meant is having something like this since this function is not limited
> to the driver scope and using it from other components would imply relying on the
> assumption: 'ep_mem_offset == sizeof(struct ffa_mem_region)'. We will also have to validate
> that the `ep_mem_offset` doesn't point outside of the mailbox designated buffer.
> 

Sure, we can extend the function itself or add addition helper to get the
functionality you are looking for the validation.

> ---
> diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> index 81e603839c4a..62d67dae8b70 100644
> --- a/include/linux/arm_ffa.h
> +++ b/include/linux/arm_ffa.h
> @@ -445,7 +445,7 @@ ffa_mem_desc_offset(struct ffa_mem_region *buf, int count, u32 ffa_version)
>         if (!FFA_MEM_REGION_HAS_EP_MEM_OFFSET(ffa_version))
>                 offset += offsetof(struct ffa_mem_region, ep_mem_offset);
>         else
> -               offset += sizeof(struct ffa_mem_region);
> +               offset += buf->ep_mem_offset;
>
>         return offset;
>  }
> ---
> 
> And then move `ffa_mem_region_additional_setup` to be called earlier before `ffa_mem_desc_offset`:
> (so that it can setup the value for ep_mem_offset)
> 
> ---
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index f2f94d4d533e..66de59c88aff 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -691,6 +691,8 @@ ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
>         mem_region->flags = args->flags;
>         mem_region->sender_id = drv_info->vm_id;
>         mem_region->attributes = ffa_memory_attributes_get(func_id);
> +
> +       ffa_mem_region_additional_setup(drv_info->version, mem_region);

Ah this could do the trick. I need to check if all the usages are covered
though.

>         composite_offset = ffa_mem_desc_offset(buffer, args->nattrs,
>                                                drv_info->version);
>  
> @@ -708,7 +710,6 @@ ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
>         }
>         mem_region->handle = 0;
>         mem_region->ep_count = args->nattrs;
> -       ffa_mem_region_additional_setup(drv_info->version, mem_region);
> ---
> 
> > 
> > > 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.
> 
> Sure, please let me know if you think this is wrong. I might have misunderstood it. 
> 

Nope, the patch helped to understand it quicker. Thanks for that.

> > 
> > > 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.
> > 
> 
> Sorry for not explaining it, in pKVM model we don't trust the host kernel so
> we can assume that everything that doesn't pass the hypervisor validation(in
> this case the ff-a memory transaction) can be a potential attack that wants
> to compromise EL2.
> 

I am aware of the principle in general, but this example with different offset
can't be assumed as comprised host if the offset + size is well within the
Tx buffer size boundaries. That should be the way for you to cross check for
any compromise IHMO.

-- 
Regards,
Sudeep