[PATCH 0/4] firmware: arm_scmi: Fix protocol parsing and validation

[PATCH 0/4] firmware: arm_scmi: Fix protocol parsing and validation

[Sudeep Holla]

Hi,

This series fixes a few SCMI protocol parsing and validation issues found
while checking the driver message layouts against the SCMI specification.

The first patch fixes a clear response width mismatch in SENSOR_CONFIG_GET,
where the driver requested a 4-byte response but read it as a 64-bit value.

The next two patches harden notification parsing for variable-sized payloads.
BASE_ERROR_EVENT and SENSOR_UPDATE both carry counted trailing arrays, so the
received payload size must be validated before copying or parsing those
entries.

The final patch fixes Powercap protocol v2 helpers to validate the domain id
before accessing cached per-domain state.

No functional change is intended for well-formed SCMI responses.

Cc: Cristian Marussi <cristian.marussi@arm.com>
Cc: arm-scmi@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>

Regards,
Sudeep

---
Sudeep Holla (4):
      firmware: arm_scmi: Read sensor config as 32-bit value
      firmware: arm_scmi: Validate BASE_ERROR_EVENT payload size
      firmware: arm_scmi: Validate SENSOR_UPDATE payload size
      firmware: arm_scmi: Validate Powercap domains before state access

 drivers/firmware/arm_scmi/base.c     | 15 +++++++++++++--
 drivers/firmware/arm_scmi/powercap.c | 25 ++++++++++++++++++-------
 drivers/firmware/arm_scmi/sensors.c  | 12 ++++++++++--
 3 files changed, 41 insertions(+), 11 deletions(-)
---
base-commit: d0deb8265c8551343c5c155aaded6b56639e3820
change-id: 20260517-scmi_fixes-211563fc2ff2


-- 
Regards,
Sudeep

[PATCH 1/4] firmware: arm_scmi: Read sensor config as 32-bit value

[Sudeep Holla]

The SENSOR_CONFIG_GET response contains a 32-bit sensor_config field,
and the xfer is initialized with a 4-byte RX buffer. Reading it with
get_unaligned_le64() can consume bytes past the returned payload.

Use get_unaligned_le32() to match the protocol layout and the allocated
response size.

Fixes: 7b83c5f41088 ("firmware: arm_scmi: Add SCMI v3.0 sensor configuration support")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_scmi/sensors.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index 882d55f987d2..836c294a9f42 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -793,7 +793,7 @@ static int scmi_sensor_config_get(const struct scmi_protocol_handle *ph,
 	if (!ret) {
 		struct scmi_sensor_info *s = si->sensors + sensor_id;
 
-		*sensor_config = get_unaligned_le64(t->rx.buf);
+		*sensor_config = get_unaligned_le32(t->rx.buf);
 		s->sensor_config = *sensor_config;
 	}
 

-- 
2.43.0

[PATCH 2/4] firmware: arm_scmi: Validate BASE_ERROR_EVENT payload size

[Sudeep Holla]

BASE_ERROR_EVENT carries a variable number of message reports,
with the count encoded in error_status. The notification parser used
that count without checking whether the received payload contained all
reported entries.

Reject truncated payloads before copying the report array.

Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_scmi/base.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index 4df2620e3c5d..d08a4f6a9ced 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -325,6 +325,8 @@ static void *scmi_base_fill_custom_report(const struct scmi_protocol_handle *ph,
 					  void *report, u32 *src_id)
 {
 	int i;
+	u32 error_status;
+	size_t expected_sz;
 	const struct scmi_base_error_notify_payld *p = payld;
 	struct scmi_base_error_report *r = report;
 
@@ -338,10 +340,19 @@ static void *scmi_base_fill_custom_report(const struct scmi_protocol_handle *ph,
 	if (evt_id != SCMI_EVENT_BASE_ERROR_EVENT || sizeof(*p) < payld_sz)
 		return NULL;
 
+	expected_sz = sizeof(p->agent_id) + sizeof(p->error_status);
+	if (payld_sz < expected_sz)
+		return NULL;
+
 	r->timestamp = timestamp;
 	r->agent_id = le32_to_cpu(p->agent_id);
-	r->fatal = IS_FATAL_ERROR(le32_to_cpu(p->error_status));
-	r->cmd_count = ERROR_CMD_COUNT(le32_to_cpu(p->error_status));
+	error_status = le32_to_cpu(p->error_status);
+	r->fatal = IS_FATAL_ERROR(error_status);
+	r->cmd_count = ERROR_CMD_COUNT(error_status);
+	expected_sz += r->cmd_count * sizeof(p->msg_reports[0]);
+	if (payld_sz < expected_sz)
+		return NULL;
+
 	for (i = 0; i < r->cmd_count; i++)
 		r->reports[i] = le64_to_cpu(p->msg_reports[i]);
 	*src_id = 0;

-- 
2.43.0

[PATCH 3/4] firmware: arm_scmi: Validate SENSOR_UPDATE payload size

[Sudeep Holla]

SENSOR_UPDATE carries one or more sensor readings after the fixed
notification header. The parser derives the expected reading count
from the sensor description, but it did not verify that the received
payload contains those entries before parsing them.

Reject truncated update notifications before reading the variable
array.

Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_scmi/sensors.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index 836c294a9f42..b14bb1146356 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -1072,12 +1072,15 @@ scmi_sensor_fill_custom_report(const struct scmi_protocol_handle *ph,
 	case SCMI_EVENT_SENSOR_UPDATE:
 	{
 		int i;
+		size_t expected_sz;
 		struct scmi_sensor_info *s;
 		const struct scmi_sensor_update_notify_payld *p = payld;
 		struct scmi_sensor_update_report *r = report;
 		struct sensors_info *sinfo = ph->get_priv(ph);
 
-		/* payld_sz is variable for this event */
+		if (payld_sz < sizeof(*p))
+			break;
+
 		r->sensor_id = le32_to_cpu(p->sensor_id);
 		if (r->sensor_id >= sinfo->num_sensors)
 			break;
@@ -1091,6 +1094,11 @@ scmi_sensor_fill_custom_report(const struct scmi_protocol_handle *ph,
 		 * readings defined for this sensor or 1 for scalar sensors.
 		 */
 		r->readings_count = s->num_axis ?: 1;
+		expected_sz = sizeof(*p) + r->readings_count *
+			      sizeof(p->readings[0]);
+		if (payld_sz < expected_sz)
+			break;
+
 		for (i = 0; i < r->readings_count; i++)
 			scmi_parse_sensor_readings(&r->readings[i],
 						   &p->readings[i]);

-- 
2.43.0

[PATCH 4/4] firmware: arm_scmi: Validate Powercap domains before state access

[Sudeep Holla]

Powercap protocol v2 keeps local enable and last-cap state per
domain. Some public operations indexed that state before checking that
the supplied domain id was valid, and cap_enable_get() updated it even
when cap_get() failed.

Validate the domain before touching the per-domain state and only
refresh cached enable state after a successful cap_get().

Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_scmi/powercap.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/arm_scmi/powercap.c b/drivers/firmware/arm_scmi/powercap.c
index ab9733f4458b..eb5c35cad026 100644
--- a/drivers/firmware/arm_scmi/powercap.c
+++ b/drivers/firmware/arm_scmi/powercap.c
@@ -453,10 +453,14 @@ static int scmi_powercap_cap_set(const struct scmi_protocol_handle *ph,
 		return -EINVAL;
 
 	/* Just log the last set request if acting on a disabled domain */
-	if (PROTOCOL_REV_MAJOR(ph->version) >= 0x2 &&
-	    !pi->states[domain_id].enabled) {
-		pi->states[domain_id].last_pcap = power_cap;
-		return 0;
+	if (PROTOCOL_REV_MAJOR(ph->version) >= 0x2) {
+		if (!scmi_powercap_dom_info_get(ph, domain_id))
+			return -EINVAL;
+
+		if (!pi->states[domain_id].enabled) {
+			pi->states[domain_id].last_pcap = power_cap;
+			return 0;
+		}
 	}
 
 	return __scmi_powercap_cap_set(ph, pi, domain_id,
@@ -637,6 +641,9 @@ static int scmi_powercap_cap_enable_set(const struct scmi_protocol_handle *ph,
 	if (PROTOCOL_REV_MAJOR(ph->version) < 0x2)
 		return -EINVAL;
 
+	if (!scmi_powercap_dom_info_get(ph, domain_id))
+		return -EINVAL;
+
 	if (enable == pi->states[domain_id].enabled)
 		return 0;
 
@@ -678,16 +685,20 @@ static int scmi_powercap_cap_enable_get(const struct scmi_protocol_handle *ph,
 	if (PROTOCOL_REV_MAJOR(ph->version) < 0x2)
 		return 0;
 
+	if (!scmi_powercap_dom_info_get(ph, domain_id))
+		return -EINVAL;
+
 	/*
 	 * Report always real platform state; platform could have ignored
 	 * a previous disable request. Default true on any error.
 	 */
 	ret = scmi_powercap_cap_get(ph, domain_id, &power_cap);
-	if (!ret)
+	if (!ret) {
 		*enable = !!power_cap;
 
-	/* Update internal state with current real platform state */
-	pi->states[domain_id].enabled = *enable;
+		/* Update internal state with current real platform state */
+		pi->states[domain_id].enabled = *enable;
+	}
 
 	return 0;
 }

-- 
2.43.0