* [PATCH] iio: adc: pac1934: validate ACPI _DSM return type and bounds
@ 2026-10-26 23:09 Tristan Madani
2026-10-27 1:09 ` Jonathan Cameron
2026-10-27 7:40 ` Andy Shevchenko
0 siblings, 2 replies; 3+ messages in thread
From: Tristan Madani @ 2026-10-26 23:09 UTC (permalink / raw)
To: Marius Cristea, Jonathan Cameron
Cc: David Lechner, Nuno Sa, Andy Shevchenko, linux-iio, linux-kernel,
stable, tristan
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")
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;
--
2.47.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: adc: pac1934: validate ACPI _DSM return type and bounds
2026-10-26 23:09 [PATCH] iio: adc: pac1934: validate ACPI _DSM return type and bounds Tristan Madani
@ 2026-10-27 1:09 ` Jonathan Cameron
2026-10-27 7:40 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2026-10-27 1:09 UTC (permalink / raw)
To: Tristan Madani
Cc: Marius Cristea, David Lechner, Nuno Sa, Andy Shevchenko,
linux-iio, linux-kernel, stable, tristan
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;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: adc: pac1934: validate ACPI _DSM return type and bounds
2026-10-26 23:09 [PATCH] iio: adc: pac1934: validate ACPI _DSM return type and bounds Tristan Madani
2026-10-27 1:09 ` Jonathan Cameron
@ 2026-10-27 7:40 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-10-27 7:40 UTC (permalink / raw)
To: Tristan Madani
Cc: Marius Cristea, Jonathan Cameron, David Lechner, Nuno Sa,
Andy Shevchenko, linux-iio, linux-kernel, stable, tristan
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-27 7:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-26 23:09 [PATCH] iio: adc: pac1934: validate ACPI _DSM return type and bounds Tristan Madani
2026-10-27 1:09 ` Jonathan Cameron
2026-10-27 7:40 ` Andy Shevchenko
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®