[PATCH v2] firmware: arm_ffa: honor descriptor size in PARTITION_INFO_GET_REGS
Sudeep Holla
sudeep.holla at kernel.org
Sun May 17 10:28:05 PDT 2026
On Fri, May 15, 2026 at 10:44:32AM -0700, Jamie Nguyen wrote:
> __ffa_partition_info_get_regs() walks the response with a hardcoded
> 24-byte stride (regs += 3) even though the SPMC tells us the actual
> per-descriptor size via PARTITION_INFO_SZ in x2[63:48]. The size is
> read into buf_sz and then thrown away.
>
> That works while every SPMC returns the FF-A v1.1 layout, but it falls
> apart against a v1.3 SPMC returning the 48-byte descriptor. The loop
> strides over half a descriptor at a time and ends up parsing every
> other entry from a slice of two adjacent ones.
>
> The FF-A spec (v1.2, section 18.5) says that the producer should
> report the descriptor size, and the consumer is supposed to stride by
> that size and ignore any trailing fields it doesn't understand. The
> non-REGS path (__ffa_partition_info_get) does this already, and the
> REGS path should match.
>
> Use buf_sz for the stride, and bail out with -EINVAL if the SPMC
> reports something we can't safely walk.
>
> Fixes: ba85c644ac8d ("firmware: arm_ffa: Add support for FFA_PARTITION_INFO_GET_REGS")
> Signed-off-by: Jamie Nguyen <jamien at nvidia.com>
> ---
> Changes in v2:
> - Rebase onto linux-next; reuse the
> FFA_PART_INFO_GET_REGS_{REGS_PER_DESC,MAX_DESC} macros it added instead of
> introducing new ones.
> - Return -EINVAL instead of -EPROTO to match surrounding checks.
> - Update Fixes: tag to the commit that introduced the hardcoded stride.
> ---
> drivers/firmware/arm_ffa/driver.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index b9f17fda7243..38ae4476e864 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -374,9 +374,23 @@ __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
> return -EINVAL;
>
> tag = UUID_INFO_TAG(partition_info.a2);
> +
> + /*
> + * Per FF-A v1.2 section 18.5 the SPMC reports the per-
> + * descriptor size and consumers must stride by that size,
> + * reading only the fields they understand and ignoring any
> + * trailing ones. Reject sizes that cannot hold the v1.1
> + * fields parsed below, are not u64-aligned, or whose total
> + * payload would walk past the x3..x17 window (e.g. a v1.3
> + * 48-byte descriptor with nr_desc > 2).
> + */
I am not sure if this above note is necessary.
> buf_sz = PARTITION_INFO_SZ(partition_info.a2);
> - if (buf_sz > sizeof(*buffer))
> - buf_sz = sizeof(*buffer);
> + if (buf_sz < FFA_PART_INFO_GET_REGS_REGS_PER_DESC * sizeof(u64) ||
> + buf_sz % sizeof(u64) ||
> + nr_desc * (buf_sz / sizeof(u64)) >
> + FFA_PART_INFO_GET_REGS_MAX_DESC *
> + FFA_PART_INFO_GET_REGS_REGS_PER_DESC)
> + return -EINVAL;
>
The logic above is correct but bit hard to read, I am thinking of splitting
this
> regs = (void *)&partition_info.a3;
> for (idx = 0; idx < nr_desc; idx++, buf++) {
> @@ -395,7 +409,7 @@ __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
> buf->exec_ctxt = PART_INFO_EXEC_CXT(val);
> buf->properties = PART_INFO_PROPERTIES(val);
> uuid_copy(&buf->uuid, &uuid_regs.uuid);
> - regs += 3;
> + regs += buf_sz / sizeof(u64);
> }
> start_idx = cur_idx + 1;
Is it OK if I adjust something like below patch ?
Regards,
Sudeep
-->8
diff --git c/drivers/firmware/arm_ffa/driver.c w/drivers/firmware/arm_ffa/driver.c
index fb97b01c282a..54984e1b9741 100644
--- c/drivers/firmware/arm_ffa/driver.c
+++ w/drivers/firmware/arm_ffa/driver.c
@@ -329,11 +329,9 @@ __ffa_partition_info_get(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
#define PART_INFO_EXEC_CXT_MASK GENMASK(31, 16)
#define PART_INFO_PROPS_MASK GENMASK(63, 32)
#define FFA_PART_INFO_GET_REGS_FIRST_REG 3
-#define FFA_PART_INFO_GET_REGS_REGS_PER_DESC 3
-#define FFA_PART_INFO_GET_REGS_MAX_DESC \
- (((sizeof(ffa_value_t) / sizeof_field(ffa_value_t, a0)) - \
- FFA_PART_INFO_GET_REGS_FIRST_REG) / \
- FFA_PART_INFO_GET_REGS_REGS_PER_DESC)
+#define FFA_PART_INFO_GET_REGS_MIN_REGS_PER_DESC 3
+#define FFA_PART_INFO_GET_REGS_NUM_REGS \
+ (sizeof(ffa_value_t) / sizeof_field(ffa_value_t, a0))
#define PART_INFO_ID(x) ((u16)(FIELD_GET(PART_INFO_ID_MASK, (x))))
#define PART_INFO_EXEC_CXT(x) ((u16)(FIELD_GET(PART_INFO_EXEC_CXT_MASK, (x))))
#define PART_INFO_PROPERTIES(x) ((u32)(FIELD_GET(PART_INFO_PROPS_MASK, (x))))
@@ -347,7 +345,7 @@ __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
do {
__le64 *regs;
- int idx, nr_desc, buf_idx;
+ int idx, nr_desc, buf_idx, regs_per_desc, max_desc;
invoke_ffa_fn((ffa_value_t){
.a0 = FFA_PARTITION_INFO_GET_REGS,
@@ -370,8 +368,18 @@ __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
if (cur_idx < start_idx || cur_idx >= count)
return -EINVAL;
+ buf_sz = PARTITION_INFO_SZ(partition_info.a2);
+ if (buf_sz % sizeof(*regs))
+ return -EINVAL;
+
+ regs_per_desc = buf_sz / sizeof(*regs);
+ if (regs_per_desc < FFA_PART_INFO_GET_REGS_MIN_REGS_PER_DESC)
+ return -EINVAL;
+
nr_desc = cur_idx - start_idx + 1;
- if (nr_desc > FFA_PART_INFO_GET_REGS_MAX_DESC)
+ max_desc = (FFA_PART_INFO_GET_REGS_NUM_REGS -
+ FFA_PART_INFO_GET_REGS_FIRST_REG) / regs_per_desc;
+ if (nr_desc > max_desc)
return -EINVAL;
buf_idx = buf - buffer;
@@ -379,9 +387,6 @@ __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
return -EINVAL;
tag = UUID_INFO_TAG(partition_info.a2);
- buf_sz = PARTITION_INFO_SZ(partition_info.a2);
- if (buf_sz > sizeof(*buffer))
- buf_sz = sizeof(*buffer);
regs = (void *)&partition_info.a3;
for (idx = 0; idx < nr_desc; idx++, buf++) {
@@ -400,7 +405,7 @@ __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
buf->exec_ctxt = PART_INFO_EXEC_CXT(val);
buf->properties = PART_INFO_PROPERTIES(val);
uuid_copy(&buf->uuid, &uuid_regs.uuid);
- regs += 3;
+ regs += regs_per_desc;
}
start_idx = cur_idx + 1;
More information about the linux-arm-kernel
mailing list