linux-kernel - Re: [PATCH v7 03/10] iio: adc: add helpers for parsing ADC nodes

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: <Z9LQFqSweiV-zT3b@smile.fi.intel.com>
Date: Thu, 13 Mar 2025 14:31:18 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
	Jonathan Cameron <jic23@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
	Chen-Yu Tsai <wens@...e.org>,
	Jernej Skrabec <jernej.skrabec@...il.com>,
	Samuel Holland <samuel@...lland.org>,
	Hugo Villeneuve <hvilleneuve@...onoff.com>,
	Nuno Sa <nuno.sa@...log.com>, David Lechner <dlechner@...libre.com>,
	Javier Carrasco <javier.carrasco.cruz@...il.com>,
	Guillaume Stols <gstols@...libre.com>,
	Dumitru Ceclan <mitrutzceclan@...il.com>,
	Trevor Gamblin <tgamblin@...libre.com>,
	Matteo Martelli <matteomartelli3@...il.com>,
	Alisa-Dariana Roman <alisadariana@...il.com>,
	Ramona Alexandra Nechita <ramona.nechita@...log.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
	linux-renesas-soc@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH v7 03/10] iio: adc: add helpers for parsing ADC nodes

On Thu, Mar 13, 2025 at 09:18:18AM +0200, Matti Vaittinen wrote:
> There are ADC ICs which may have some of the AIN pins usable for other
> functions. These ICs may have some of the AIN pins wired so that they
> should not be used for ADC.
> 
> (Preferred?) way for marking pins which can be used as ADC inputs is to
> add corresponding channels@N nodes in the device tree as described in
> the ADC binding yaml.
> 
> Add couple of helper functions which can be used to retrieve the channel
> information from the device node.

...

> +int devm_iio_adc_device_alloc_chaninfo_se(struct device *dev,
> +					  const struct iio_chan_spec *template,
> +					  int max_chan_id,
> +					  struct iio_chan_spec **cs)
> +{
> +	struct iio_chan_spec *chan_array, *chan;
> +	int num_chan = 0, ret;

Unneeded assignment.

> +	num_chan = iio_adc_device_num_channels(dev);
> +	if (num_chan < 1)
> +		return num_chan;

This is really interesting code. So, if the above returns negative error code,
we return it, if it returns 0, we return success (but 0 channels)?

Shouldn't we do *cs = NULL; at the case of 0 channels if it's a success?
(Under success I assume that returned values are okay to go with, and cs in
your case will be left uninitialised or contain something we don't control.

> +	chan_array = devm_kcalloc(dev, num_chan, sizeof(*chan_array),
> +				  GFP_KERNEL);
> +	if (!chan_array)
> +		return -ENOMEM;
> +
> +	chan = &chan_array[0];
> +
> +	device_for_each_named_child_node_scoped(dev, child, "channel") {
> +		u32 ch;
> +
> +		ret = fwnode_property_read_u32(child, "reg", &ch);
> +		if (ret)
> +			return ret;

> +		if (max_chan_id != -1 && ch > max_chan_id)
> +			return -ERANGE;

Hmm... What if max_chan_id is equal to an error code?
Or in other words, why -1 is special and not all negative numbers?

Also note, you used unsigned type and compare it to int which,
in case of being negative will give promotion. The ch will not be
big enough in most cases (unless it's great than (INT_MAX + 1).

TL;DR: you have a potential integer overflow here.

> +		*chan = *template;
> +		chan->channel = ch;
> +		chan++;
> +	}
> +
> +	*cs = chan_array;
> +
> +	return num_chan;
> +}

-- 
With Best Regards,
Andy Shevchenko