Re: [RFC PATCH 3/7] firmware: arm_scmi: Add Telemetry protocol support

On Fri, Jun 20, 2025 at 11:46:15PM +0300, Dan Carpenter wrote:

On Fri, Jun 20, 2025 at 08:28:09PM +0100, Cristian Marussi wrote:

quoted

+static int
+scmi_telemetry_protocol_attributes_get(const struct scmi_protocol_handle *ph,
+				       struct telemetry_info *ti)
+{

Hi Dan,

thanks for having a look.

quoted

+	int ret;
+	struct scmi_xfer *t;
+	struct scmi_msg_resp_telemetry_protocol_attributes *resp;
+
+	ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES,
+				      0, sizeof(*resp), &t);
+	if (ret)
+		return ret;
+
+	resp = t->rx.buf;
+	ret = ph->xops->do_xfer(ph, t);
+	if (!ret) {
+		__le32 attr = resp->attributes;
+
+		ti->info.num_de = le32_to_cpu(resp->de_num);
+		ti->info.num_groups = le32_to_cpu(resp->groups_num);
+		for (int i = 0; i < SCMI_TLM_MAX_DWORD; i++)
+			ti->info.de_impl_version[i] =
+				le32_to_cpu(resp->de_implementation_rev_dword[i]);
+		ti->info.single_read_support = SUPPORTS_SINGLE_READ(attr);
+		ti->info.continuos_update_support = SUPPORTS_CONTINUOS_UPDATE(attr);
+		ti->info.per_group_config_support = SUPPORTS_PER_GROUP_CONFIG(attr);
+		ti->info.reset_support = SUPPORTS_RESET(attr);
+		ti->info.fc_support = SUPPORTS_FC(attr);
+		ti->num_shmti = le32_get_bits(attr, GENMASK(15, 0));
+		/* Allocate DEs descriptors */
+		ti->info.des = devm_kcalloc(ph->dev, ti->info.num_de,
+					    sizeof(*ti->info.des), GFP_KERNEL);
+		if (!ti->info.des)
+			ret = -ENOMEM;
+
+		/* Allocate DE GROUPS descriptors */
+		ti->info.des_groups = devm_kcalloc(ph->dev, ti->info.num_groups,
+						   sizeof(*ti->info.des_groups),
+						   GFP_KERNEL);
+		if (!ti->info.des_groups)
+			ret = -ENOMEM;

It the allocation fails we need to jump to the ->xfer_put

quoted

+
+		for (int i = 0; i < ti->info.num_groups; i++)
+			ti->info.des_groups[i].id = i;

otherwise it leads to a NULL dereference.

Indeed...I will fix.

There are a few more of this bugs around probably being an RFC...bots also are
complaining a lot at this series indeed :D

Thanks,
Cristian