linux-kernel - Re: [PATCH 07/10] firmware: arm_scmi: Add System Telemetry ioctls support

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] [day] [month] [year] [list]

Message-ID: <20251020173034.00005c15@huawei.com>
Date: Mon, 20 Oct 2025 17:30:34 +0100
From: Jonathan Cameron <jonathan.cameron@...wei.com>
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>,
	<dan.carpenter@...aro.org>, <d-gole@...com>, <souvik.chakravarty@....com>
Subject: Re: [PATCH 07/10] firmware: arm_scmi: Add System Telemetry ioctls
 support

On Thu, 25 Sep 2025 21:35:51 +0100
Cristian Marussi <cristian.marussi@....com> wrote:

> Extend the filesystem based interface with special 'control' file that can
> be used to configure and retrieve SCMI Telemetry data in binary form using
> the alternative ioctls-based ABI described in uapi/linux/scmi.h.
Why you say alternative.  Why do you need both?

That's in the cover letter but I'd put something here as well.
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@....com>
> ---
>  .../firmware/arm_scmi/scmi_system_telemetry.c | 402 ++++++++++++++++++
>  1 file changed, 402 insertions(+)
> 
> diff --git a/drivers/firmware/arm_scmi/scmi_system_telemetry.c b/drivers/firmware/arm_scmi/scmi_system_telemetry.c
> index 2fec465b0f33..f591aad10302 100644
> --- a/drivers/firmware/arm_scmi/scmi_system_telemetry.c
> +++ b/drivers/firmware/arm_scmi/scmi_system_telemetry.c

> +static long scmi_tlm_des_read_ioctl(const struct scmi_tlm_inode *tlmi,
> +				    unsigned long arg, bool single,
> +				    bool is_group)
> +{
> +	const struct scmi_tlm_setup *tsp = tlmi->tsp;
> +	void * __user uptr = (void * __user)arg;
> +	struct scmi_tlm_data_read bulk, *bulk_ptr;
> +	int ret, grp_id = SCMI_TLM_GRP_INVALID;
> +
> +	if (copy_from_user(&bulk, uptr, sizeof(bulk)))
> +		return -EFAULT;
> +
> +	bulk_ptr = kzalloc(struct_size(bulk_ptr, samples, bulk.num_samples),

__free() would help here.

> +			   GFP_KERNEL);
> +	if (!bulk_ptr)
> +		return -ENOMEM;
> +
> +	if (is_group) {
> +		const struct scmi_telemetry_group *grp = tlmi->priv;
> +
> +		grp_id = grp->info->id;
> +	}
> +
> +	bulk_ptr->num_samples = bulk.num_samples;
> +	if (!single)
> +		ret = tsp->ops->des_bulk_read(tsp->ph, grp_id,
> +					      &bulk_ptr->num_samples,
> +			  (struct scmi_telemetry_de_sample *)bulk_ptr->samples);
> +	else
> +		ret = tsp->ops->des_sample_get(tsp->ph, grp_id,
> +					       &bulk_ptr->num_samples,
> +			  (struct scmi_telemetry_de_sample *)bulk_ptr->samples);

That is very unusual code alignment.  Drag 2 lines above left to match one line above.

> +	if (ret)
> +		goto out;
> +
> +	if (copy_to_user(uptr, bulk_ptr, sizeof(*bulk_ptr) +
> +			 bulk_ptr->num_samples * sizeof(bulk_ptr->samples[0])))
> +		ret = -EFAULT;
> +
> +out:
> +	kfree(bulk_ptr);
> +
> +	return ret;
> +}