From: Hans de Goede <hdegoede@redhat.com>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Andy Shevchenko" <andy@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>,
James Harmison <jharmison@redhat.com>,
platform-driver-x86@vger.kernel.org, stable@vger.kernel.org
Subject: [PATCH v2 1/3] platform/x86: panasonic-laptop: Fix SINF array out of bounds accesses
Date: Mon, 9 Sep 2024 13:32:25 +0200 [thread overview]
Message-ID: <20240909113227.254470-1-hdegoede@redhat.com> (raw)
The panasonic laptop code in various places uses the SINF array with index
values of 0 - SINF_CUR_BRIGHT(0x0d) without checking that the SINF array
is big enough.
Not all panasonic laptops have this many SINF array entries, for example
the Toughbook CF-18 model only has 10 SINF array entries. So it only
supports the AC+DC brightness entries and mute.
Check that the SINF array has a minimum size which covers all AC+DC
brightness entries and refuse to load if the SINF array is smaller.
For higher SINF indexes hide the sysfs attributes when the SINF array
does not contain an entry for that attribute, avoiding show()/store()
accessing the array out of bounds and add bounds checking to the probe()
and resume() code accessing these.
Fixes: e424fb8cc4e6 ("panasonic-laptop: avoid overflow in acpi_pcc_hotkey_add()")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Some panasonic laptops have only 10 SINF entries. Change the required
minimum SQTY to SQTY > SINF_DC_CUR_BRIGHT and avoid the driver accessing
higher indexes by adding num_sifr checks.
---
drivers/platform/x86/panasonic-laptop.c | 49 ++++++++++++++++++++-----
1 file changed, 39 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index cf845ee1c7b1..39044119d2a6 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -773,6 +773,24 @@ static DEVICE_ATTR_RW(dc_brightness);
static DEVICE_ATTR_RW(current_brightness);
static DEVICE_ATTR_RW(cdpower);
+static umode_t pcc_sysfs_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct acpi_device *acpi = to_acpi_device(dev);
+ struct pcc_acpi *pcc = acpi_driver_data(acpi);
+
+ if (attr == &dev_attr_mute.attr)
+ return (pcc->num_sifr > SINF_MUTE) ? attr->mode : 0;
+
+ if (attr == &dev_attr_eco_mode.attr)
+ return (pcc->num_sifr > SINF_ECO_MODE) ? attr->mode : 0;
+
+ if (attr == &dev_attr_current_brightness.attr)
+ return (pcc->num_sifr > SINF_CUR_BRIGHT) ? attr->mode : 0;
+
+ return attr->mode;
+}
+
static struct attribute *pcc_sysfs_entries[] = {
&dev_attr_numbatt.attr,
&dev_attr_lcdtype.attr,
@@ -787,8 +805,9 @@ static struct attribute *pcc_sysfs_entries[] = {
};
static const struct attribute_group pcc_attr_group = {
- .name = NULL, /* put in device directory */
- .attrs = pcc_sysfs_entries,
+ .name = NULL, /* put in device directory */
+ .attrs = pcc_sysfs_entries,
+ .is_visible = pcc_sysfs_is_visible,
};
@@ -941,12 +960,15 @@ static int acpi_pcc_hotkey_resume(struct device *dev)
if (!pcc)
return -EINVAL;
- acpi_pcc_write_sset(pcc, SINF_MUTE, pcc->mute);
- acpi_pcc_write_sset(pcc, SINF_ECO_MODE, pcc->eco_mode);
+ if (pcc->num_sifr > SINF_MUTE)
+ acpi_pcc_write_sset(pcc, SINF_MUTE, pcc->mute);
+ if (pcc->num_sifr > SINF_ECO_MODE)
+ acpi_pcc_write_sset(pcc, SINF_ECO_MODE, pcc->eco_mode);
acpi_pcc_write_sset(pcc, SINF_STICKY_KEY, pcc->sticky_key);
acpi_pcc_write_sset(pcc, SINF_AC_CUR_BRIGHT, pcc->ac_brightness);
acpi_pcc_write_sset(pcc, SINF_DC_CUR_BRIGHT, pcc->dc_brightness);
- acpi_pcc_write_sset(pcc, SINF_CUR_BRIGHT, pcc->current_brightness);
+ if (pcc->num_sifr > SINF_CUR_BRIGHT)
+ acpi_pcc_write_sset(pcc, SINF_CUR_BRIGHT, pcc->current_brightness);
return 0;
}
@@ -963,8 +985,12 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
num_sifr = acpi_pcc_get_sqty(device);
- if (num_sifr < 0 || num_sifr > 255) {
- pr_err("num_sifr out of range");
+ /*
+ * pcc->sinf is expected to at least have the AC+DC brightness entries.
+ * Accesses to higher SINF entries are checked against num_sifr.
+ */
+ if (num_sifr <= SINF_DC_CUR_BRIGHT || num_sifr > 255) {
+ pr_err("num_sifr %d out of range %d - 255\n", num_sifr, SINF_DC_CUR_BRIGHT + 1);
return -ENODEV;
}
@@ -1020,11 +1046,14 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
acpi_pcc_write_sset(pcc, SINF_STICKY_KEY, 0);
pcc->sticky_key = 0;
- pcc->eco_mode = pcc->sinf[SINF_ECO_MODE];
- pcc->mute = pcc->sinf[SINF_MUTE];
pcc->ac_brightness = pcc->sinf[SINF_AC_CUR_BRIGHT];
pcc->dc_brightness = pcc->sinf[SINF_DC_CUR_BRIGHT];
- pcc->current_brightness = pcc->sinf[SINF_CUR_BRIGHT];
+ if (pcc->num_sifr > SINF_MUTE)
+ pcc->mute = pcc->sinf[SINF_MUTE];
+ if (pcc->num_sifr > SINF_ECO_MODE)
+ pcc->eco_mode = pcc->sinf[SINF_ECO_MODE];
+ if (pcc->num_sifr > SINF_CUR_BRIGHT)
+ pcc->current_brightness = pcc->sinf[SINF_CUR_BRIGHT];
/* add sysfs attributes */
result = sysfs_create_group(&device->dev.kobj, &pcc_attr_group);
--
2.46.0
next reply other threads:[~2024-09-09 11:32 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-09 11:32 Hans de Goede [this message]
2024-09-09 11:32 ` [PATCH v2 2/3] platform/x86: panasonic-laptop: Allocate 1 entry extra in the sinf array Hans de Goede
2024-09-09 12:06 ` [PATCH v2 1/3] platform/x86: panasonic-laptop: Fix SINF array out of bounds accesses Andy Shevchenko
2024-09-09 12:11 ` Hans de Goede
2024-09-09 12:46 ` Andy Shevchenko
2024-09-09 17:54 ` Ilpo Järvinen
2024-09-09 18:40 ` Ilpo Järvinen
2024-09-10 14:26 ` Konstantin Ryabitsev
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=20240909113227.254470-1-hdegoede@redhat.com \
--to=hdegoede@redhat.com \
--cc=andy@kernel.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jharmison@redhat.com \
--cc=platform-driver-x86@vger.kernel.org \
--cc=stable@vger.kernel.org \
/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