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

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 26 Oct 2026 23:09:10 +0000
Tristan Madani <tristmd@gmail.com> 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].
> 
> Fixes: 0fb528c8255b ("iio: adc: adding support for PAC193x")
I should have mentioned this for earlier patches.  This is
to my eyes not a fix, it is hardening against buggy firmware.

Now I'm fine with doing such hardening but we don't want to
suggest it is something folk should definitely backport.

> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
>  drivers/iio/adc/pac1934.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/pac1934.c b/drivers/iio/adc/pac1934.c
> index de59dc27c4405..43c4705c27be7 100644
> --- a/drivers/iio/adc/pac1934.c
> +++ b/drivers/iio/adc/pac1934.c
> @@ -1101,8 +1101,13 @@ static int pac1934_acpi_parse_channel_config(struct i2c_client *client,
>  	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;
> +	}
>  
> -	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) {
>  		idx = i / 2;
>  		info->labels[idx] =
>  			devm_kmemdup(dev, rez->package.elements[i].string.pointer,
> @@ -1130,7 +1135,13 @@ static int pac1934_acpi_parse_channel_config(struct i2c_client *client,
>  		return 0;
>  	}
>  
> -	for (i = 0; i < rez->package.count; i++) {
> +	if (rez->type != ACPI_TYPE_PACKAGE) {
> +		ACPI_FREE(rez);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < rez->package.count &&
> +	     i < PAC1934_MAX_NUM_CHANNELS; i++) {
>  		idx = i;
>  		info->shunts[idx] = rez->package.elements[i].integer.value;
>  		info->active_channels[idx] = (info->shunts[idx] != 0);
> @@ -1141,6 +1152,10 @@ static int pac1934_acpi_parse_channel_config(struct i2c_client *client,
>  	rez = acpi_evaluate_dsm(handle, &guid, 1, PAC1934_ACPI_GET_BIPOLAR_SETTINGS, NULL);
>  	if (!rez)
>  		return -EINVAL;
> +	if (rez->type != ACPI_TYPE_PACKAGE || rez->package.count < 1) {
> +		ACPI_FREE(rez);
> +		return -EINVAL;
> +	}
>  
>  	bi_dir_mask = rez->package.elements[0].integer.value;
>  	info->bi_dir[0] = ((bi_dir_mask & (1 << 3)) | (bi_dir_mask & (1 << 7))) != 0;
> @@ -1153,6 +1168,10 @@ static int pac1934_acpi_parse_channel_config(struct i2c_client *client,
>  	rez = acpi_evaluate_dsm(handle, &guid, 1, PAC1934_ACPI_GET_SAMP, NULL);
>  	if (!rez)
>  		return -EINVAL;
> +	if (rez->type != ACPI_TYPE_PACKAGE || rez->package.count < 1) {
> +		ACPI_FREE(rez);
> +		return -EINVAL;
> +	}
>  
>  	info->sample_rate_value = rez->package.elements[0].integer.value;
>