Re: [PATCH] iio: adc: xilinx-ams: fix out-of-bounds accesses when

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Thu, Oct 22, 2026 at 04:23:34AM +0530, Arnav Kapoor wrote:

 Subject: [PATCH] iio: adc: xilinx-ams: fix out-of-bounds accesses when

when what?..

Please, read what you about to send. Most likely you used AI for all this and
hence people tend to stop checking all the crap it produces...

> ams_parse_firmware() allocates room for ARRAY_SIZE(ams_ps_channels) +
> ARRAY_SIZE(ams_pl_channels) + ARRAY_SIZE(ams_ctrl_channels) = 51
> channel specs and lets ams_init_module() fill them for the AMS node and
> each of its children, without telling it how much room is left.
> 
> For the PL-SYSMON node, ams_get_ext_chan() appends one channel per
> "channel@N" child after the 10 fixed PL channels. The binding allows
> reg values 20..50, so a PL node can have up to 31 such children, i.e.
> up to 41 PL channels, while only 31 are budgeted for it. Together with
> the 13 PS and 7 AMS control channels, this writes past the end of the
> buffer. Since the modules are filled in device tree order, the fixed
> channel blocks copied after the PL node can overflow as well.
> 
> ams_get_ext_chan() also only checks the upper bound of reg. ext_chan is
> unsigned, so a reg below 20 makes it wrap and ams_pl_channels[] is read
> far out of bounds.
> 
> Pass the remaining capacity down to ams_init_module() and
> ams_get_ext_chan() and fail with -ENOSPC when a module does not fit,
> instead of writing past the buffer. Also reject reg values below 20, as
> the binding requires.

> This was previously reported by Weigang He, but the thread stalled
> before the changes were picked up.

Convert this paragraph to Reported-by, Closes, and respective Link tags.

...

> +		ret = ams_get_ext_chan(fwnode, channels, num_channels,
> +					max_channels);

Perhaps a single line?

...

>  	if (fwnode_device_is_available(fwnode)) {
> -		ret = ams_init_module(indio_dev, fwnode, ams_channels);
> +		ret = ams_init_module(indio_dev, fwnode, ams_channels,
> +				      (int)(ams_size - num_channels));

Why do we need casting? is ams_init_module() is aware of negative numbers?
If so, it can also be aware of some "big" number.

>  		if (ret < 0)
>  			return ret;

...

> static int ams_parse_firmware(struct iio_dev *indio_dev)

>  	device_for_each_child_node_scoped(dev, child) {
> -		ret = ams_init_module(indio_dev, child, ams_channels + num_channels);
> +		ret = ams_init_module(indio_dev, child,
> +				      ams_channels + num_channels,
> +				      (int)(ams_size - num_channels));

Ditto.

>  		if (ret < 0)
>  			return ret;

-- 
With Best Regards,
Andy Shevchenko