Re: [PATCH] iio: adc: pac1934: validate ACPI _DSM return type and bounds

[Andy Shevchenko]

On Mon, Oct 26, 2026 at 11:09:10PM +0000, Tristan Madani wrote:
> pac1934_acpi_parse_channel_config() calls acpi_evaluate_dsm() four
> times to read channel configuration but does not validate the returned
> object type or array bounds:
> 
>   1. No check that the returned object is ACPI_TYPE_PACKAGE.  If
>      firmware returns a different type, accessing rez->package.count
>      and rez->package.elements on a non-package object produces
>      unpredictable values.
> 
>   2. The loop bounds use rez->package.count directly without capping
>      to PAC1934_MAX_NUM_CHANNELS (4).  A package with more elements
>      than expected causes writes past the fixed-size labels[],
>      shunts[], and active_channels[] arrays.
> 
>   3. The bipolar and sample rate DSM calls access elements[0] without
>      verifying package.count >= 1.
> 
> Add ACPI_TYPE_PACKAGE validation after each acpi_evaluate_dsm() call,
> cap loop iterations to PAC1934_MAX_NUM_CHANNELS, and verify package
> count >= 1 before accessing elements[0].

...

>  	rez = acpi_evaluate_dsm(handle, &guid, 0, PAC1934_ACPI_GET_NAMES_AND_MOHMS_VALS, NULL);
>  	if (!rez)
>  		return -EINVAL;
> +	if (rez->type != ACPI_TYPE_PACKAGE) {
> +		ACPI_FREE(rez);
> +		return -EINVAL;
> +	}

So, what's wrong with acpi_evaluate_dsm_typed()?
Ditto for other cases below.

...

> -	for (i = 0; i < rez->package.count; i += 2) {
> +	for (i = 0; i + 1 < rez->package.count &&
> +	     i / 2 < PAC1934_MAX_NUM_CHANNELS; i += 2) {

This is ugly and inefficient. Why can't you just check count to be even number?
Or choose the limit in the loop making it an even number?

	for (i = 0; i < (rez->package.count & ~1); i += 2) {

>  		idx = i / 2;
>  		info->labels[idx] =
>  			devm_kmemdup(dev, rez->package.elements[i].string.pointer,

>  	}

-- 
With Best Regards,
Andy Shevchenko