Re: [PATCH v6 3/6] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation

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

Hi Mostafa,

> From: Sebastian Ene <[email protected]>
> 
> 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 <[email protected]>
> Reviewed-by: Sudeep Holla <[email protected]>
> Signed-off-by: Mostafa Saleh <[email protected]>
> ---
>  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