On Wed, May 27, 2026 at 03:02:33PM +0000, Mostafa Saleh wrote:

Hi Mostafa,

From: Sebastian Ene sebastianene@google.com

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`. [@Mostafa Harden error checking]

Fixes: 113580530ee7 ("firmware: arm_ffa: Update memory descriptor to support v1.1 format") Signed-off-by: Sebastian Ene sebastianene@google.com Reviewed-by: Sudeep Holla sudeep.holla@kernel.org Signed-off-by: Mostafa Saleh smostafa@google.com

---

drivers/firmware/arm_ffa/driver.c | 16 +++++++++++----- include/linux/arm_ffa.h | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 059e2aae7ca0..bed4bd48963f 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -703,19 +703,26 @@ 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;
• u32 emad_size = ffa_emad_size_get(drv_info->version);

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 + sizeof(*composite) > max_fragsize)

• return -ENXIO;
  

for (idx = 0; idx < args->nattrs; idx++) {

• ep_mem_access = buffer +
  

• 	ffa_mem_desc_offset(buffer, idx, drv_info->version);
  

• memset(ep_mem_access, 0, ffa_emad_size_get(drv_info->version));
  

• ep_offset = ffa_mem_desc_offset(buffer, idx, drv_info->version);
  

• if (ep_offset + emad_size > max_fragsize)
  

• 	return -ENXIO;
  

Looking between v3 (the version that I sent) and this version, you account correctly for the emad_size which depends on the FF-A version (thus the introduced call to ffa_emad_size_get()).

I think the only remaining bit here is making sure that we don't overflow in `ep_offset + emad_size`.

• ep_mem_access = buffer + ep_offset;
  

• memset(ep_mem_access, 0, emad_size);
  

  ep_mem_access->receiver = args->attrs[idx].receiver; ep_mem_access->attrs = args->attrs[idx].attrs; ep_mem_access->composite_off = composite_offset;

@@ -725,7 +732,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.746.g67dd491aae-goog

Thanks, Sebastian

On Wed, May 27, 2026 at 03:02:34PM +0000, Mostafa Saleh wrote:

Sashiko (locally) reports out of bound write possiblity if SPMD returns an invalid data.

While SPMD is considered trusted, pKVM does some basic checks, for offset to be less than or equal len.

However, that is incorrect as even if the offset is smaller than len pKVM can still access out of bound memory in the next ffa_host_unshare_ranges().

Split this check into 2: 1- Check that the fixed portion of the descriptor fits. 2- After getting reg, check the variable array size addr_range_cnt fits.

Also, drop the WARN_ONs as that will panic the kernel and in the next checks there are no WARNs, so that makes it consistent.

Fixes: 0a9f15fd5674 ("KVM: arm64: pkvm: Add support for fragmented FF-A descriptors") Signed-off-by: Mostafa Saleh smostafa@google.com

---

arch/arm64/kvm/hyp/nvhe/ffa.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c index 1af722771178..b6cf9ad82e12 100644 --- a/arch/arm64/kvm/hyp/nvhe/ffa.c +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c @@ -607,8 +607,8 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res, * check that we end up with something that doesn't look _completely_ * bogus. */

• if (WARN_ON(offset > len ||

•     fraglen > KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE)) {
  

• if (offset + CONSTITUENTS_OFFSET(0) > len ||

•    fraglen > KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE) {
  

  ret = FFA_RET_ABORTED; ffa_rx_release(res); goto out_unlock;

@@ -641,6 +641,11 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res, goto out_unlock; reg = (void *)buf + offset;

• if (offset + CONSTITUENTS_OFFSET(reg->addr_range_cnt) > len) {

• ret = FFA_RET_ABORTED;
  

• goto out_unlock;
  

• }

I wonder if we should do this check first and then mem_reclaim, the rational being that if the check fails the page will be lost forever.

``` reg = (void *)buf + offset; if (offset + CONSTITUENTS_OFFSET(reg->addr_range_cnt) > len) { ret = FFA_RET_ABORTED; goto out_unlock; }

ffa_mem_reclaim(res, handle_lo, handle_hi, flags); if (res->a0 != FFA_SUCCESS) goto out_unlock; ```

/* If the SPMD was happy, then we should be too. */ WARN_ON(ffa_host_unshare_ranges(reg->constituents, reg->addr_range_cnt)); -- 2.54.0.746.g67dd491aae-goog

Sebastian

On Wed, May 27, 2026 at 03:02:35PM +0000, Mostafa Saleh wrote:

From: Sebastian Ene sebastianene@google.com

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.

Also, fix one hardcoded sizeof(struct ffa_mem_region_attributes) that should be replaced ffa_emad_size_get() for compatibility with FFA v1.0.

[@Mostafa, Add missing call to ffa_rx_release() and use fraglen as the max buffer size as it is the only intialised part]

Fixes: 42fb33dde42b ("KVM: arm64: Use FF-A 1.1 with pKVM") Signed-off-by: Sebastian Ene sebastianene@google.com Signed-off-by: Mostafa Saleh smostafa@google.com

---

arch/arm64/kvm/hyp/nvhe/ffa.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c index b6cf9ad82e12..a12e01883314 100644 --- a/arch/arm64/kvm/hyp/nvhe/ffa.c +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c @@ -476,10 +476,10 @@ static void __do_ffa_mem_xfer(const u64 func_id, DECLARE_REG(u32, fraglen, ctxt, 2); DECLARE_REG(u64, addr_mbz, ctxt, 3); DECLARE_REG(u32, npages_mbz, ctxt, 4);

• u32 offset, nr_ranges, checked_offset, em_mem_access_off; 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; int ret = 0;

if (addr_mbz || npages_mbz || fraglen > len || @@ -489,7 +489,7 @@ static void __do_ffa_mem_xfer(const u64 func_id, }

Hi Mostafa,

if (fraglen < sizeof(struct ffa_mem_region) +

•       sizeof(struct ffa_mem_region_attributes)) {
  

•       ffa_emad_size_get(hyp_ffa_version)) {
  

  ret = FFA_RET_INVALID_PARAMETERS; goto out; }

Here sashiko complains about a compatibility break with v1.0 even though it is not from this change. This is because the size of sizeof(struct ffa_mem_region) was 36 bytes in v1.0 and now it is 48.

This check can cause "perfectly valid v1.0 memory transfers to be rejected" (from sashiko).

Other than that this looks good. I will spin a new version of this series.

@@ -508,8 +508,13 @@ 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 ((u64)em_mem_access_off + ffa_emad_size_get(hyp_ffa_version) > fraglen) {

• 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;

@@ -574,9 +579,9 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res, DECLARE_REG(u32, handle_lo, ctxt, 1); DECLARE_REG(u32, handle_hi, ctxt, 2); DECLARE_REG(u32, flags, ctxt, 3);

• u32 offset, len, fraglen, fragoff, em_mem_access_off; struct ffa_mem_region_attributes *ep_mem_access; struct ffa_composite_mem_region *reg;

• u32 offset, len, fraglen, fragoff; struct ffa_mem_region *buf; int ret = 0; u64 handle;

@@ -599,8 +604,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 ((u64)em_mem_access_off + ffa_emad_size_get(hyp_ffa_version) > fraglen) {

• ret = FFA_RET_INVALID_PARAMETERS;
  

• ffa_rx_release(res);
  

• 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.746.g67dd491aae-goog

Thanks, Sebastian

On Wed, May 27, 2026 at 03:02:36PM +0000, Mostafa Saleh wrote:

Hi Mostafa,

At the moment we only check that the size of the range is page aligned, and truncate the address to the page boundary. This make an assumption that TZ will do the same.

However, it might decide to use the extra offset of the neighbour page at the end, which is valid under FFA if NS is using larger page size.

Harden this check by also checking that the base address is aligned and reject it otherwise.

Fixes: 436090001776 ("KVM: arm64: Handle FFA_MEM_SHARE calls from the host") Signed-off-by: Mostafa Saleh smostafa@google.com

---

arch/arm64/kvm/hyp/nvhe/ffa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c index a12e01883314..daf0e328c847 100644 --- a/arch/arm64/kvm/hyp/nvhe/ffa.c +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c @@ -352,7 +352,7 @@ static u32 __ffa_host_share_ranges(struct ffa_mem_region_addr_range *ranges, u64 sz = (u64)range->pg_cnt * FFA_PAGE_SIZE; u64 pfn = hyp_phys_to_pfn(range->address);

• if (!PAGE_ALIGNED(sz))
  

• if (!PAGE_ALIGNED(sz | range->address))
  break;
  

Should we do the pfn assignment after the check ?

if (__pkvm_host_share_ffa(pfn, sz / PAGE_SIZE))

@@ -372,7 +372,7 @@ static u32 __ffa_host_unshare_ranges(struct ffa_mem_region_addr_range *ranges, u64 sz = (u64)range->pg_cnt * FFA_PAGE_SIZE; u64 pfn = hyp_phys_to_pfn(range->address);

• if (!PAGE_ALIGNED(sz))
  

• if (!PAGE_ALIGNED(sz | range->address))
  break;
  

The same here

if (__pkvm_host_unshare_ffa(pfn, sz / PAGE_SIZE))

-- 2.54.0.746.g67dd491aae-goog

Thanks, Sebastian