linux-kernel - Re: [RFC PATCH 3/7] firmware: arm_scmi: Add Telemetry protocol 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aFwBdjWQv6urIgq1@pluto>
Date: Wed, 25 Jun 2025 15:02:30 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Cristian Marussi <cristian.marussi@....com>,
	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 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:
> > +static int
> > +scmi_telemetry_protocol_attributes_get(const struct scmi_protocol_handle *ph,
> > +				       struct telemetry_info *ti)
> > +{

Hi Dan,

thanks for having a look.

> > +	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
> 
> > +
> > +		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