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

[Andy Shevchenko]

On Sat, Sep 26, 2026 at 03:54:56PM +1000, Weigang He wrote:
> 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 -EINVAL when a module does not fit,
> instead of writing past the buffer. Also reject reg values below 20, as
> the binding requires.
> 
> Found by static analysis tool CodeQL.

...

>  	fwnode_for_each_child_node(chan_node, child) {

It seems better to switch to _scoped() variant at some point.

>  		ret = fwnode_property_read_u32(child, "reg", &reg);
> -		if (ret || reg > AMS_PL_MAX_EXT_CHANNEL + 30)
> +		if (ret || reg < 30 - AMS_PL_MAX_FIXED_CHANNEL ||
> +		    reg > AMS_PL_MAX_EXT_CHANNEL + 30)
>  			continue;
>  
> +		if (num_channels >= max_channels) {
> +			fwnode_handle_put(child);
> +			return -EINVAL;

-ECHRNG

> +		}

...

>  	if (fwnode_device_is_compatible(fwnode, "xlnx,zynqmp-ams-ps")) {
> +		if (max_channels < ARRAY_SIZE(ams_ps_channels))
> +			return -EINVAL;

ENOSPC?

...

>  	} else if (fwnode_device_is_compatible(fwnode, "xlnx,zynqmp-ams-pl")) {
> +		if (max_channels < AMS_PL_MAX_FIXED_CHANNEL)
> +			return -EINVAL;

Ditto.

...

>  	} else if (fwnode_device_is_compatible(fwnode, "xlnx,zynqmp-ams")) {
> +		if (max_channels < ARRAY_SIZE(ams_ctrl_channels))
> +			return -EINVAL;

Ditto.

-- 
With Best Regards,
Andy Shevchenko