Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] firmware: arm_ffa: honor descriptor size in PARTITION_INFO_GET_REGS
@ 2026-05-13  3:28 Jamie Nguyen
  2026-05-13 17:15 ` Sudeep Holla
  0 siblings, 1 reply; 3+ messages in thread
From: Jamie Nguyen @ 2026-05-13  3:28 UTC (permalink / raw)
  To: sudeep.holla; +Cc: linux-arm-kernel, linux-kernel, jamien

__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 -EPROTO if the SPMC
reports something we can't safely walk.

Fixes: 7bc0f589c81d ("firmware: arm_ffa: Fix big-endian support in __ffa_partition_info_regs_get()")
Signed-off-by: Jamie Nguyen <jamien@nvidia.com>
---
 drivers/firmware/arm_ffa/driver.c | 35 ++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index c72ee4756585..b712e8a03dab 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -321,6 +321,22 @@ __ffa_partition_info_get(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
 #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))))
+
+/*
+ * FF-A v1.2 section 13.9 Table 13.40: registers x3..x17 carry the partition
+ * descriptors, i.e. 15 u64 of payload per FFA_PARTITION_INFO_GET_REGS call.
+ */
+#define FFA_PART_INFO_REGS_PAYLOAD_U64	15
+
+/*
+ * FF-A v1.1 partition information descriptor (FF-A v1.2 section 6.2.1
+ * Table 6.1): id (2) + exec_ctxt (2) + properties (4) + UUID (16) = 24
+ * bytes. This is the minimum size the SPMC must report; the kernel reads
+ * exactly these fields and ignores any trailing ones per the forward-
+ * compatibility rules in FF-A v1.2 section 18.5.
+ */
+#define FFA_PART_INFO_DESC_V1_1_SZ	24
+
 static int
 __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
 			      struct ffa_partition_info *buffer, int num_parts)
@@ -353,8 +369,21 @@ __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
 		cur_idx = CURRENT_INDEX(partition_info.a2);
 		tag = UUID_INFO_TAG(partition_info.a2);
 		buf_sz = PARTITION_INFO_SZ(partition_info.a2);
-		if (buf_sz > sizeof(*buffer))
-			buf_sz = sizeof(*buffer);
+
+		/*
+		 * Per FF-A v1.2 section 18.5 the SPMC reports its per-
+		 * descriptor size and consumers must stride by that size,
+		 * consuming only the fields they understand and ignoring any
+		 * trailing ones. Reject sizes that cannot hold the v1.1 fields
+		 * read below, are not u64-aligned, or would overrun the
+		 * x3..x17 window.
+		 */
+		if (buf_sz < FFA_PART_INFO_DESC_V1_1_SZ ||
+		    buf_sz % sizeof(u64))
+			return -EPROTO;
+		if ((cur_idx - start_idx + 1) * buf_sz >
+		    FFA_PART_INFO_REGS_PAYLOAD_U64 * sizeof(u64))
+			return -EPROTO;
 
 		regs = (void *)&partition_info.a3;
 		for (idx = 0; idx < cur_idx - start_idx + 1; idx++, buf++) {
@@ -373,7 +402,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);
 		}
 		prev_idx = cur_idx;
 

base-commit: 38edeaf4dcf3d8381ba801e494ba03179c145c70
-- 
2.34.1



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

* Re: [PATCH] firmware: arm_ffa: honor descriptor size in PARTITION_INFO_GET_REGS
  2026-05-13  3:28 [PATCH] firmware: arm_ffa: honor descriptor size in PARTITION_INFO_GET_REGS Jamie Nguyen
@ 2026-05-13 17:15 ` Sudeep Holla
  2026-05-13 19:48   ` Jamie Nguyen
  0 siblings, 1 reply; 3+ messages in thread
From: Sudeep Holla @ 2026-05-13 17:15 UTC (permalink / raw)
  To: Jamie Nguyen; +Cc: linux-arm-kernel, linux-kernel, Sudeep Holla

On Tue, May 12, 2026 at 08:28:00PM -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 -EPROTO if the SPMC
> reports something we can't safely walk.
> 

Can you check if the issue is addressed in -next by:
Commit 3974ea193840 ("firmware: arm_ffa: Bound PARTITION_INFO_GET_REGS copies")

-- 
Regards,
Sudeep


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

* Re: [PATCH] firmware: arm_ffa: honor descriptor size in PARTITION_INFO_GET_REGS
  2026-05-13 17:15 ` Sudeep Holla
@ 2026-05-13 19:48   ` Jamie Nguyen
  0 siblings, 0 replies; 3+ messages in thread
From: Jamie Nguyen @ 2026-05-13 19:48 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org



> On May 13, 2026, at 10:15 AM, Sudeep Holla <sudeep.holla@kernel.org> wrote:
> 
> On Tue, May 12, 2026 at 08:28:00PM -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 -EPROTO if the SPMC
>> reports something we can't safely walk.
>> 
> 
> Can you check if the issue is addressed in -next by:
> Commit 3974ea193840 ("firmware: arm_ffa: Bound PARTITION_INFO_GET_REGS copies")

Thanks for the pointer.  I tested 3974ea193840 on the same hardware
that reproduces the bug, but the descriptor-stride issue is still
present.

The relevant loop at the end of __ffa_partition_info_get_regs()
still has:

	buf_sz = PARTITION_INFO_SZ(partition_info.a2);
	if (buf_sz > sizeof(*buffer))
		buf_sz = sizeof(*buffer);
	...
	for (idx = 0; idx < nr_desc; idx++, buf++) {
		...
		regs += 3;  /* bug is here */
	}

With 48-byte descriptors the SPMC returns nr_desc = 2 per call,
which passes the new MAX_DESC = 5 bound, and the loop then runs
the 24-byte stride over 48-byte entries.

Here are the test results on the same SBIOS / same hardware:

  unpatched:               5 partitions enumerated, two with
                           garbled UUIDs and partition_ids
  3974ea193840 alone:      same misparsed enumeration
  3974ea193840 + my patch: 6 partitions, all entries correct

So the two fixes look complementary rather than overlapping.

> -- 
> Regards,
> Sudeep



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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13  3:28 [PATCH] firmware: arm_ffa: honor descriptor size in PARTITION_INFO_GET_REGS Jamie Nguyen
2026-05-13 17:15 ` Sudeep Holla
2026-05-13 19:48   ` Jamie Nguyen

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