linux-kernel - Re: [RFC PATCH 4/7] firmware: arm_scmi: Add System Telemetry driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f2c0a0d9-6a1b-42c7-a180-e04e357dbbb5@suswa.mountain>
Date: Sat, 21 Jun 2025 00:27:03 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Cristian Marussi <cristian.marussi@....com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	arm-scmi@...r.kernel.org, sudeep.holla@....com,
	james.quinlan@...adcom.com, f.fainelli@...il.com,
	vincent.guittot@...aro.org, etienne.carriere@...com,
	peng.fan@....nxp.com, michal.simek@....com, quic_sibis@...cinc.com,
	d-gole@...com, souvik.chakravarty@....com
Subject: Re: [RFC PATCH 4/7] firmware: arm_scmi: Add System Telemetry driver

On Fri, Jun 20, 2025 at 08:28:10PM +0100, Cristian Marussi wrote:
> +//TODO Review available interval show
> +#define BUF_SZ	1024
> +static inline ssize_t
> +__available_update_show(char *buf,
> +			const struct scmi_telemetry_update_interval *intervals)
> +{
> +	int len = 0, num_intervals = intervals->num;
> +	char available[BUF_SZ];
> +
> +	for (int i = 0; i < num_intervals; i++) {
> +		len += scnprintf(available + len, BUF_SZ - len, "%u ",
> +				 intervals->update_intervals[i]);
> +	}
> +
> +	available[len - 1] = '\0';

No need.  scnprintf() will already have put a NUL terminator there.
Unless num_intervals <= 0 in which case this will corrupt memory.

> +
> +	return sysfs_emit(buf, "%s\n", available);
> +}

[ snip ]

> +static int scmi_telemetry_groups_initialize(struct device *dev,
> +					    struct scmi_tlm_instance *ti)
> +{
> +	int ret;
> +
> +	if (ti->info->num_groups == 0)
> +		return 0;
> +
> +	ret = scmi_telemetry_dev_register(&ti->groups_dev, &ti->dev, "groups");
> +	if (ret)
> +		return ret;
> +
> +	for (int i = 0; i < ti->info->num_groups; i++) {
> +		const struct scmi_telemetry_group *grp = &ti->info->des_groups[i];
> +		struct scmi_tlm_grp_dev *gdev;
> +		char name[16];
> +
> +		gdev = devm_kzalloc(dev, sizeof(*gdev), GFP_KERNEL);
> +		if (!gdev) {
> +			ret = -ENOMEM;
> +			goto err;
> +		}
> +
> +		gdev->tsp = ti->tsp;
> +		gdev->grp = grp;
> +		gdev->dev.groups = scmi_grp_groups;
> +
> +		snprintf(name, 8, "%d", grp->id);

s/8/sizeof(name)/?

> +		ret = scmi_telemetry_dev_register(&gdev->dev,
> +						  &ti->groups_dev, name);
> +		if (ret)
> +			goto err;
> +
> +		if (ti->info->per_group_config_support) {
> +			sysfs_add_file_to_group(&gdev->dev.kobj,
> +						&dev_attr_grp_current_update.attr,
> +						NULL);
> +			sysfs_add_file_to_group(&gdev->dev.kobj,
> +						&dev_attr_grp_intervals_discrete.attr,
> +						NULL);
> +			sysfs_add_file_to_group(&gdev->dev.kobj,
> +						&dev_attr_grp_available_intervals.attr,
> +						NULL);
> +		}
> +	}
> +
> +	dev_info(dev, "Found %d Telemetry GROUPS resources.\n",
> +		 ti->info->num_groups);
> +
> +	return 0;
> +
> +err:
> +	scmi_telemetry_dev_unregister(&ti->groups_dev);
> +
> +	return ret;
> +}
> +
> +static int scmi_telemetry_des_initialize(struct device *dev,
> +					 struct scmi_tlm_instance *ti)
> +{
> +	int ret;
> +
> +	ret = scmi_telemetry_dev_register(&ti->des_dev, &ti->dev, "des");
> +	if (ret)
> +		return ret;
> +
> +	for (int i = 0; i < ti->info->num_de; i++) {
> +		const struct scmi_telemetry_de *de = ti->info->des[i];
> +		struct scmi_tlm_de_dev *tdev;
> +		char name[16];
> +
> +		tdev = devm_kzalloc(dev, sizeof(*tdev), GFP_KERNEL);
> +		if (!tdev) {
> +			ret = -ENOMEM;
> +			goto err;
> +		}
> +
> +		tdev->tsp = ti->tsp;
> +		tdev->de = de;
> +		tdev->dev.groups = scmi_des_groups;
> +
> +		/*XXX What about of ID/name digits-length used ? */
> +		snprintf(name, 8, "0x%04X", de->id);

s/8/sizeof(name)/?

regards,
dan carpenter

> +		ret = scmi_telemetry_dev_register(&tdev->dev,
> +						  &ti->des_dev, name);
> +		if (ret)
> +			goto err;
> +
> +		if (de->name)
> +			sysfs_add_file_to_group(&tdev->dev.kobj,
> +						&dev_attr_name.attr, NULL);
> +		if (de->tstamp_support) {
> +			sysfs_add_file_to_group(&tdev->dev.kobj,
> +						&dev_attr_tstamp_exp.attr,
> +						NULL);
> +			sysfs_add_file_to_group(&tdev->dev.kobj,
> +						&dev_attr_tstamp_enable.attr,
> +						NULL);
> +		}
> +	}
> +
> +	dev_info(dev, "Found %d Telemetry DE resources.\n",
> +		 ti->info->num_de);
> +
> +	return 0;
> +
> +err:
> +	scmi_telemetry_dev_unregister(&ti->des_dev);
> +
> +	return ret;
> +}