From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Tristan Madani <tristmd@gmail.com>
Cc: Marius Cristea <marius.cristea@microchip.com>,
Jonathan Cameron <jic23@kernel.org>,
David Lechner <dlechner@baylibre.com>,
Nuno Sa <nuno.sa@analog.com>, Andy Shevchenko <andy@kernel.org>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, tristan@talencesecurity.com
Subject: Re: [PATCH] iio: adc: pac1934: validate ACPI _DSM return type and bounds
Date: Tue, 27 Oct 2026 09:40:04 +0200 [thread overview]
Message-ID: <auBVVIuIPhZZf9OH@ashevche-desk.local> (raw)
In-Reply-To: <20261026230910.2569782-1-tristan@talencesecurity.com>
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
prev parent reply other threads:[~2026-10-27 7:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-26 23:09 Tristan Madani
2026-10-27 1:09 ` Jonathan Cameron
2026-10-27 7:40 ` Andy Shevchenko [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=auBVVIuIPhZZf9OH@ashevche-desk.local \
--to=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marius.cristea@microchip.com \
--cc=nuno.sa@analog.com \
--cc=stable@vger.kernel.org \
--cc=tristan@talencesecurity.com \
--cc=tristmd@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®