[PATCH v3 1/2] firmware: arm_ffa: Fix Endpoint Memory Access Descriptor offset calculation

[Sudeep Holla]

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