Re: [PATCH v4] iio: adc: pac1934: validate ACPI _DSM return type and bounds
From: Andy Shevchenko
Date: Fri Oct 30 2026 - 05:49:56 EST
On Thu, Oct 29, 2026 at 04:56:02PM +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.
>
> Use acpi_evaluate_dsm_typed() with ACPI_TYPE_PACKAGE to validate the
> return type at evaluation time. Cap loop iterations to
> PAC1934_MAX_NUM_CHANNELS, and verify package count >= 1 before
> accessing elements[0].
...
> static int pac1934_acpi_parse_channel_config(struct i2c_client *client,
> struct pac1934_chip_info *info)
> {
> - acpi_handle handle;
> - union acpi_object *rez;
> struct device *dev = &client->dev;
> unsigned short bi_dir_mask;
> - int idx, i;
> + union acpi_object *rez;
> + acpi_handle handle;
> + unsigned int count;
> guid_t guid;
> + int idx, i;
No need to touch lines that are unrelated to your change. Just make sure that
your change tries to keep reversed xmas tree order as much as possible.
...
> - for (i = 0; i < rez->package.count; i += 2) {
> + count = min(rez->package.count & ~1u, PAC1934_MAX_NUM_CHANNELS * 2);
What is the purpose of 'u' suffix? In case if you want to keep that 32-bit
value this might be even wrong (I haven't checked the promotion rules for the
values with suffix, though).
> + for (i = 0; i < count; i += 2) {
--
With Best Regards,
Andy Shevchenko