Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] arm_ffa, KVM: Fix FF-A emad offset calculations
@ 2026-05-12 12:44 Sebastian Ene
  2026-05-12 12:44 ` [PATCH v3 1/2] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation Sebastian Ene
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sebastian Ene @ 2026-05-12 12:44 UTC (permalink / raw)
  To: catalin.marinas, maz, oupton, sudeep.holla, will
  Cc: joey.gouly, korneld, kvmarm, linux-arm-kernel, linux-kernel,
	android-kvm, mrigendra.chaubey, perlarsen, sebastianene,
	suzuki.poulose, vdonnefort, yuzenghui

Hi all,

This series fixes the Endpoint Memory Access Descriptor (EMAD) offset calculations
and adds the necessary bounds checks for both the core FF-A driver and the pKVM
hypervisor.

Prior to FF-A version 1.1, the memory region header didn't specify an explicit offset
for the EMADs, leading to the assumption that they immediately follow the header.
However, from v1.1 onwards, the specification dictates using the `ep_mem_offset` field
to determine the start of the memory access array.

The patches in this series address this by:
1. Updating the core `arm_ffa` firmware driver to correctly calculate the descriptor
   offset using `ep_mem_offset` rather than defaulting to `sizeof(struct ffa_mem_region)`.
   It also introduces bounds checking against `max_fragsize`.
2. Enhancing the pKVM hypervisor validation logic to no longer strictly enforce that
   the descriptor strictly follows the header, aligning it with the driver behavior
   and the FF-A specification, while also ensuring the offset falls within the mailbox
   buffer bounds.

Changelog
#########

v2 -> this:
- Fixed typo in nvhe/ffa.c (missing sizeof)

v1 -> v2:
- For pKVM, removed the strict placement enforcement for `ep_mem_offset` as it is not
  compliant with the spec, and avoids making assumptions about the driver's memory
  layout.

Link to:
########

v2: https://lore.kernel.org/all/20260430160241.1934777-1-sebastianene@google.com/
v1: https://lore.kernel.org/all/ae9KN9nkOgDYJcGP@google.com/T/#t

Sebastian Ene (2):
  firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset
    calculation
  KVM: arm64: Validate the offset to the mem access descriptor

 arch/arm64/kvm/hyp/nvhe/ffa.c     | 24 ++++++++++++++++++------
 drivers/firmware/arm_ffa/driver.c | 14 ++++++++++----
 include/linux/arm_ffa.h           |  2 +-
 3 files changed, 29 insertions(+), 11 deletions(-)

-- 
2.54.0.563.g4f69b47b94-goog



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation
  2026-05-12 12:44 [PATCH v3 0/2] arm_ffa, KVM: Fix FF-A emad offset calculations Sebastian Ene
@ 2026-05-12 12:44 ` Sebastian Ene
  2026-05-13 13:34   ` Mostafa Saleh
  2026-05-12 12:44 ` [PATCH v3 2/2] KVM: arm64: Validate the offset to the mem access descriptor Sebastian Ene
  2026-05-13 17:23 ` [PATCH v3 0/2] arm_ffa, KVM: Fix FF-A emad offset calculations Sudeep Holla
  2 siblings, 1 reply; 7+ messages in thread
From: Sebastian Ene @ 2026-05-12 12:44 UTC (permalink / raw)
  To: catalin.marinas, maz, oupton, sudeep.holla, will
  Cc: joey.gouly, korneld, kvmarm, linux-arm-kernel, linux-kernel,
	android-kvm, mrigendra.chaubey, perlarsen, sebastianene,
	suzuki.poulose, vdonnefort, yuzenghui

Use the descriptor's `ep_mem_offset` to calculate the start of the endpoint
memory access array and to comply with the FF-A spec instead of defaulting
to `sizeof(struct ffa_mem_region)`.
This requires moving `ffa_mem_region_additional_setup()` earlier in the setup
flow.
Also, add sanity checks to ensure the calculated descriptor offsets do not
exceed `max_fragsize`.

Signed-off-by: Sebastian Ene <sebastianene@google.com>
---
 drivers/firmware/arm_ffa/driver.c | 14 ++++++++++----
 include/linux/arm_ffa.h           |  2 +-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index eb2782848283..56b166290b24 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -685,18 +685,25 @@ ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
 	struct ffa_composite_mem_region *composite;
 	struct ffa_mem_region_addr_range *constituents;
 	struct ffa_mem_region_attributes *ep_mem_access;
-	u32 idx, frag_len, length, buf_sz = 0, num_entries = sg_nents(args->sg);
+	u32 idx, frag_len, length, buf_sz = 0, num_entries = sg_nents(args->sg), ep_offset;
 
 	mem_region->tag = args->tag;
 	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);
+	if (composite_offset > max_fragsize - sizeof(struct ffa_composite_mem_region))
+		return -ENXIO;
 
 	for (idx = 0; idx < args->nattrs; idx++) {
-		ep_mem_access = buffer +
-			ffa_mem_desc_offset(buffer, idx, drv_info->version);
+		ep_offset = ffa_mem_desc_offset(buffer, idx, drv_info->version);
+		if (ep_offset > max_fragsize - sizeof(struct ffa_mem_region_attributes))
+			return -ENXIO;
+
+		ep_mem_access = buffer + ep_offset;
 		ep_mem_access->receiver = args->attrs[idx].receiver;
 		ep_mem_access->attrs = args->attrs[idx].attrs;
 		ep_mem_access->composite_off = composite_offset;
@@ -708,7 +715,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);
 
 	composite = buffer + composite_offset;
 	composite->total_pg_cnt = ffa_get_num_pages_sg(args->sg);
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;
 }
-- 
2.54.0.563.g4f69b47b94-goog



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] KVM: arm64: Validate the offset to the mem access descriptor
  2026-05-12 12:44 [PATCH v3 0/2] arm_ffa, KVM: Fix FF-A emad offset calculations Sebastian Ene
  2026-05-12 12:44 ` [PATCH v3 1/2] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation Sebastian Ene
@ 2026-05-12 12:44 ` Sebastian Ene
  2026-05-13 13:53   ` Mostafa Saleh
  2026-05-13 17:23 ` [PATCH v3 0/2] arm_ffa, KVM: Fix FF-A emad offset calculations Sudeep Holla
  2 siblings, 1 reply; 7+ messages in thread
From: Sebastian Ene @ 2026-05-12 12:44 UTC (permalink / raw)
  To: catalin.marinas, maz, oupton, sudeep.holla, will
  Cc: joey.gouly, korneld, kvmarm, linux-arm-kernel, linux-kernel,
	android-kvm, mrigendra.chaubey, perlarsen, sebastianene,
	suzuki.poulose, vdonnefort, yuzenghui

Prevent the pKVM hypervisor from making assumptions that the
endpoint memory access descriptor (EMAD) comes right after the
FF-A memory region header.
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 from 1.1 onward:
Type | Field name | Offset
[ Header | ffa_mem_region  | 0
  EMAD 1 | ffa_mem_region_attributes) | ffa_mem_region.ep_mem_offset
]
Verify that the offset to the first endpoint memory access descriptor
is within the mailbox buffer bounds.

Signed-off-by: Sebastian Ene <sebastianene@google.com>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 1af722771178..34927bc1239b 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -479,7 +479,7 @@ static void __do_ffa_mem_xfer(const u64 func_id,
 	struct ffa_mem_region_attributes *ep_mem_access;
 	struct ffa_composite_mem_region *reg;
 	struct ffa_mem_region *buf;
-	u32 offset, nr_ranges, checked_offset;
+	u32 offset, nr_ranges, checked_offset, em_mem_access_off;
 	int ret = 0;
 
 	if (addr_mbz || npages_mbz || fraglen > len ||
@@ -508,8 +508,14 @@ static void __do_ffa_mem_xfer(const u64 func_id,
 	buf = hyp_buffers.tx;
 	memcpy(buf, host_buffers.tx, fraglen);
 
-	ep_mem_access = (void *)buf +
-			ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
+	em_mem_access_off = ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
+	if (em_mem_access_off >
+	    KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE - sizeof(struct ffa_mem_region_attributes)) {
+		ret = FFA_RET_INVALID_PARAMETERS;
+		goto out_unlock;
+	}
+
+	ep_mem_access = (void *)buf + em_mem_access_off;
 	offset = ep_mem_access->composite_off;
 	if (!offset || buf->ep_count != 1 || buf->sender_id != HOST_FFA_ID) {
 		ret = FFA_RET_INVALID_PARAMETERS;
@@ -576,7 +582,7 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res,
 	DECLARE_REG(u32, flags, ctxt, 3);
 	struct ffa_mem_region_attributes *ep_mem_access;
 	struct ffa_composite_mem_region *reg;
-	u32 offset, len, fraglen, fragoff;
+	u32 offset, len, fraglen, fragoff, em_mem_access_off;
 	struct ffa_mem_region *buf;
 	int ret = 0;
 	u64 handle;
@@ -599,8 +605,14 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res,
 	len = res->a1;
 	fraglen = res->a2;
 
-	ep_mem_access = (void *)buf +
-			ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
+	em_mem_access_off = ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
+	if (em_mem_access_off >
+	    KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE - sizeof(struct ffa_mem_region_attributes)) {
+		ret = FFA_RET_INVALID_PARAMETERS;
+		goto out_unlock;
+	}
+
+	ep_mem_access = (void *)buf + em_mem_access_off;
 	offset = ep_mem_access->composite_off;
 	/*
 	 * We can trust the SPMD to get this right, but let's at least
-- 
2.54.0.563.g4f69b47b94-goog



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation
  2026-05-12 12:44 ` [PATCH v3 1/2] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation Sebastian Ene
@ 2026-05-13 13:34   ` Mostafa Saleh
  2026-05-13 17:19     ` Sudeep Holla
  0 siblings, 1 reply; 7+ messages in thread
From: Mostafa Saleh @ 2026-05-13 13:34 UTC (permalink / raw)
  To: Sebastian Ene
  Cc: catalin.marinas, maz, oupton, sudeep.holla, will, joey.gouly,
	korneld, kvmarm, linux-arm-kernel, linux-kernel, android-kvm,
	mrigendra.chaubey, perlarsen, suzuki.poulose, vdonnefort,
	yuzenghui

On Tue, May 12, 2026 at 12:44:41PM +0000, Sebastian Ene wrote:
> Use the descriptor's `ep_mem_offset` to calculate the start of the endpoint
> memory access array and to comply with the FF-A spec instead of defaulting
> to `sizeof(struct ffa_mem_region)`.
> This requires moving `ffa_mem_region_additional_setup()` earlier in the setup
> flow.
> Also, add sanity checks to ensure the calculated descriptor offsets do not
> exceed `max_fragsize`.
> 
> Signed-off-by: Sebastian Ene <sebastianene@google.com>
> ---
>  drivers/firmware/arm_ffa/driver.c | 14 ++++++++++----
>  include/linux/arm_ffa.h           |  2 +-
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index eb2782848283..56b166290b24 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -685,18 +685,25 @@ ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
>  	struct ffa_composite_mem_region *composite;
>  	struct ffa_mem_region_addr_range *constituents;
>  	struct ffa_mem_region_attributes *ep_mem_access;
> -	u32 idx, frag_len, length, buf_sz = 0, num_entries = sg_nents(args->sg);
> +	u32 idx, frag_len, length, buf_sz = 0, num_entries = sg_nents(args->sg), ep_offset;
>  
>  	mem_region->tag = args->tag;
>  	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);
> +	if (composite_offset > max_fragsize - sizeof(struct ffa_composite_mem_region))
> +		return -ENXIO;

nit: This driver seems to use sizeof() with variable name rather than
type (except for one place) so it may be good to keep that.

>  
>  	for (idx = 0; idx < args->nattrs; idx++) {
> -		ep_mem_access = buffer +
> -			ffa_mem_desc_offset(buffer, idx, drv_info->version);
> +		ep_offset = ffa_mem_desc_offset(buffer, idx, drv_info->version);
> +		if (ep_offset > max_fragsize - sizeof(struct ffa_mem_region_attributes))
> +			return -ENXIO;
> +
> +		ep_mem_access = buffer + ep_offset;
>  		ep_mem_access->receiver = args->attrs[idx].receiver;
>  		ep_mem_access->attrs = args->attrs[idx].attrs;
>  		ep_mem_access->composite_off = composite_offset;
> @@ -708,7 +715,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);
>  
>  	composite = buffer + composite_offset;
>  	composite->total_pg_cnt = ffa_get_num_pages_sg(args->sg);
> 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;

Does it make sense to also set buf->ep_mem_offset for the other
case in ffa_mem_region_additional_setup() and then add this
unconditionally here?

Thanks,
Mostafa


>  
>  	return offset;
>  }
> -- 
> 2.54.0.563.g4f69b47b94-goog
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] KVM: arm64: Validate the offset to the mem access descriptor
  2026-05-12 12:44 ` [PATCH v3 2/2] KVM: arm64: Validate the offset to the mem access descriptor Sebastian Ene
@ 2026-05-13 13:53   ` Mostafa Saleh
  0 siblings, 0 replies; 7+ messages in thread
From: Mostafa Saleh @ 2026-05-13 13:53 UTC (permalink / raw)
  To: Sebastian Ene
  Cc: catalin.marinas, maz, oupton, sudeep.holla, will, joey.gouly,
	korneld, kvmarm, linux-arm-kernel, linux-kernel, android-kvm,
	mrigendra.chaubey, perlarsen, suzuki.poulose, vdonnefort,
	yuzenghui

On Tue, May 12, 2026 at 12:44:42PM +0000, Sebastian Ene wrote:
> Prevent the pKVM hypervisor from making assumptions that the
> endpoint memory access descriptor (EMAD) comes right after the
> FF-A memory region header.
> 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 from 1.1 onward:
> Type | Field name | Offset
> [ Header | ffa_mem_region  | 0
>   EMAD 1 | ffa_mem_region_attributes) | ffa_mem_region.ep_mem_offset
> ]
> Verify that the offset to the first endpoint memory access descriptor
> is within the mailbox buffer bounds.
> 
> Signed-off-by: Sebastian Ene <sebastianene@google.com>

Reviewed-by: Mostafa Saleh <smostafa@google.com>

Thanks,
Mostafa

> ---
>  arch/arm64/kvm/hyp/nvhe/ffa.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 1af722771178..34927bc1239b 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -479,7 +479,7 @@ static void __do_ffa_mem_xfer(const u64 func_id,
>  	struct ffa_mem_region_attributes *ep_mem_access;
>  	struct ffa_composite_mem_region *reg;
>  	struct ffa_mem_region *buf;
> -	u32 offset, nr_ranges, checked_offset;
> +	u32 offset, nr_ranges, checked_offset, em_mem_access_off;
>  	int ret = 0;
>  
>  	if (addr_mbz || npages_mbz || fraglen > len ||
> @@ -508,8 +508,14 @@ static void __do_ffa_mem_xfer(const u64 func_id,
>  	buf = hyp_buffers.tx;
>  	memcpy(buf, host_buffers.tx, fraglen);
>  
> -	ep_mem_access = (void *)buf +
> -			ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
> +	em_mem_access_off = ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
> +	if (em_mem_access_off >
> +	    KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE - sizeof(struct ffa_mem_region_attributes)) {
> +		ret = FFA_RET_INVALID_PARAMETERS;
> +		goto out_unlock;
> +	}
> +
> +	ep_mem_access = (void *)buf + em_mem_access_off;
>  	offset = ep_mem_access->composite_off;
>  	if (!offset || buf->ep_count != 1 || buf->sender_id != HOST_FFA_ID) {
>  		ret = FFA_RET_INVALID_PARAMETERS;
> @@ -576,7 +582,7 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res,
>  	DECLARE_REG(u32, flags, ctxt, 3);
>  	struct ffa_mem_region_attributes *ep_mem_access;
>  	struct ffa_composite_mem_region *reg;
> -	u32 offset, len, fraglen, fragoff;
> +	u32 offset, len, fraglen, fragoff, em_mem_access_off;
>  	struct ffa_mem_region *buf;
>  	int ret = 0;
>  	u64 handle;
> @@ -599,8 +605,14 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res,
>  	len = res->a1;
>  	fraglen = res->a2;
>  
> -	ep_mem_access = (void *)buf +
> -			ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
> +	em_mem_access_off = ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
> +	if (em_mem_access_off >
> +	    KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE - sizeof(struct ffa_mem_region_attributes)) {
> +		ret = FFA_RET_INVALID_PARAMETERS;
> +		goto out_unlock;
> +	}
> +
> +	ep_mem_access = (void *)buf + em_mem_access_off;
>  	offset = ep_mem_access->composite_off;
>  	/*
>  	 * We can trust the SPMD to get this right, but let's at least
> -- 
> 2.54.0.563.g4f69b47b94-goog
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation
  2026-05-13 13:34   ` Mostafa Saleh
@ 2026-05-13 17:19     ` Sudeep Holla
  0 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2026-05-13 17:19 UTC (permalink / raw)
  To: Mostafa Saleh
  Cc: Sebastian Ene, catalin.marinas, Sudeep Holla, maz, oupton, will,
	joey.gouly, korneld, kvmarm, linux-arm-kernel, linux-kernel,
	android-kvm, mrigendra.chaubey, perlarsen, suzuki.poulose,
	vdonnefort, yuzenghui

On Wed, May 13, 2026 at 01:34:42PM +0000, Mostafa Saleh wrote:
> On Tue, May 12, 2026 at 12:44:41PM +0000, Sebastian Ene wrote:
> > Use the descriptor's `ep_mem_offset` to calculate the start of the endpoint
> > memory access array and to comply with the FF-A spec instead of defaulting
> > to `sizeof(struct ffa_mem_region)`.
> > This requires moving `ffa_mem_region_additional_setup()` earlier in the setup
> > flow.
> > Also, add sanity checks to ensure the calculated descriptor offsets do not
> > exceed `max_fragsize`.
> > 
> > Signed-off-by: Sebastian Ene <sebastianene@google.com>
> > ---
> >  drivers/firmware/arm_ffa/driver.c | 14 ++++++++++----
> >  include/linux/arm_ffa.h           |  2 +-
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> > index eb2782848283..56b166290b24 100644
> > --- a/drivers/firmware/arm_ffa/driver.c
> > +++ b/drivers/firmware/arm_ffa/driver.c
> > @@ -685,18 +685,25 @@ ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
> >  	struct ffa_composite_mem_region *composite;
> >  	struct ffa_mem_region_addr_range *constituents;
> >  	struct ffa_mem_region_attributes *ep_mem_access;
> > -	u32 idx, frag_len, length, buf_sz = 0, num_entries = sg_nents(args->sg);
> > +	u32 idx, frag_len, length, buf_sz = 0, num_entries = sg_nents(args->sg), ep_offset;
> >  
> >  	mem_region->tag = args->tag;
> >  	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);
> > +	if (composite_offset > max_fragsize - sizeof(struct ffa_composite_mem_region))
> > +		return -ENXIO;
> 
> nit: This driver seems to use sizeof() with variable name rather than
> type (except for one place) so it may be good to keep that.
> 

Agreed, +1.

> >  
> >  	for (idx = 0; idx < args->nattrs; idx++) {
> > -		ep_mem_access = buffer +
> > -			ffa_mem_desc_offset(buffer, idx, drv_info->version);
> > +		ep_offset = ffa_mem_desc_offset(buffer, idx, drv_info->version);
> > +		if (ep_offset > max_fragsize - sizeof(struct ffa_mem_region_attributes))
> > +			return -ENXIO;
> > +
> > +		ep_mem_access = buffer + ep_offset;
> >  		ep_mem_access->receiver = args->attrs[idx].receiver;
> >  		ep_mem_access->attrs = args->attrs[idx].attrs;
> >  		ep_mem_access->composite_off = composite_offset;
> > @@ -708,7 +715,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);
> >  
> >  	composite = buffer + composite_offset;
> >  	composite->total_pg_cnt = ffa_get_num_pages_sg(args->sg);
> > 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;
> 
> Does it make sense to also set buf->ep_mem_offset for the other
> case in ffa_mem_region_additional_setup() and then add this
> unconditionally here?
> 

I need to cross-check the spec, but if I vaguely recall as the name
FFA_MEM_REGION_HAS_EP_MEM_OFFSET suggests, older versions don't have that
field to use it.

-- 
Regards,
Sudeep


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] arm_ffa, KVM: Fix FF-A emad offset calculations
  2026-05-12 12:44 [PATCH v3 0/2] arm_ffa, KVM: Fix FF-A emad offset calculations Sebastian Ene
  2026-05-12 12:44 ` [PATCH v3 1/2] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation Sebastian Ene
  2026-05-12 12:44 ` [PATCH v3 2/2] KVM: arm64: Validate the offset to the mem access descriptor Sebastian Ene
@ 2026-05-13 17:23 ` Sudeep Holla
  2 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2026-05-13 17:23 UTC (permalink / raw)
  To: Sebastian Ene
  Cc: catalin.marinas, maz, oupton, Sudeep Holla, will, joey.gouly,
	korneld, kvmarm, linux-arm-kernel, linux-kernel, android-kvm,
	mrigendra.chaubey, perlarsen, suzuki.poulose, vdonnefort,
	yuzenghui

On Tue, May 12, 2026 at 12:44:40PM +0000, Sebastian Ene wrote:
> Hi all,
> 
> This series fixes the Endpoint Memory Access Descriptor (EMAD) offset calculations
> and adds the necessary bounds checks for both the core FF-A driver and the pKVM
> hypervisor.
> 
> Prior to FF-A version 1.1, the memory region header didn't specify an explicit offset
> for the EMADs, leading to the assumption that they immediately follow the header.
> However, from v1.1 onwards, the specification dictates using the `ep_mem_offset` field
> to determine the start of the memory access array.
> 
> The patches in this series address this by:
> 1. Updating the core `arm_ffa` firmware driver to correctly calculate the descriptor
>    offset using `ep_mem_offset` rather than defaulting to `sizeof(struct ffa_mem_region)`.
>    It also introduces bounds checking against `max_fragsize`.
> 2. Enhancing the pKVM hypervisor validation logic to no longer strictly enforce that
>    the descriptor strictly follows the header, aligning it with the driver behavior
>    and the FF-A specification, while also ensuring the offset falls within the mailbox
>    buffer bounds.
>

Looks good apart from the minor nits, but how do you plan to route these
changes as they are dependent for functionality but not for the build IIUC.

I don't think I have any conflicting change so far, so it can go along with
other pKVM changes. Let me know.

> Changelog
> #########
> 
> v2 -> this:
> - Fixed typo in nvhe/ffa.c (missing sizeof)
> 
> v1 -> v2:
> - For pKVM, removed the strict placement enforcement for `ep_mem_offset` as it is not
>   compliant with the spec, and avoids making assumptions about the driver's memory
>   layout.
> 
> Link to:
> ########
> 
> v2: https://lore.kernel.org/all/20260430160241.1934777-1-sebastianene@google.com/
> v1: https://lore.kernel.org/all/ae9KN9nkOgDYJcGP@google.com/T/#t
> 
> Sebastian Ene (2):
>   firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset
>     calculation
>   KVM: arm64: Validate the offset to the mem access descriptor
> 
>  arch/arm64/kvm/hyp/nvhe/ffa.c     | 24 ++++++++++++++++++------
>  drivers/firmware/arm_ffa/driver.c | 14 ++++++++++----
>  include/linux/arm_ffa.h           |  2 +-
>  3 files changed, 29 insertions(+), 11 deletions(-)
> 
> -- 
> 2.54.0.563.g4f69b47b94-goog
> 

-- 
Regards,
Sudeep


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-05-13 17:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 12:44 [PATCH v3 0/2] arm_ffa, KVM: Fix FF-A emad offset calculations Sebastian Ene
2026-05-12 12:44 ` [PATCH v3 1/2] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation Sebastian Ene
2026-05-13 13:34   ` Mostafa Saleh
2026-05-13 17:19     ` Sudeep Holla
2026-05-12 12:44 ` [PATCH v3 2/2] KVM: arm64: Validate the offset to the mem access descriptor Sebastian Ene
2026-05-13 13:53   ` Mostafa Saleh
2026-05-13 17:23 ` [PATCH v3 0/2] arm_ffa, KVM: Fix FF-A emad offset calculations Sudeep Holla

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox