From: Tristan Madani <tristmd@gmail.com>
To: even.xu@intel.com, xinpeng.sun@intel.com, jikos@kernel.org,
bentiss@kernel.org
Cc: srinivas.pandruvada@linux.intel.com, mpearson-lenovo@squebb.ca,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Tristan Madani <tristan@talencesecurity.com>,
stable@vger.kernel.org
Subject: [PATCH v2] HID: intel-thc-hid: validate ACPI buffer length before memcpy
Date: Sun, 25 Oct 2026 15:47:45 +0000 [thread overview]
Message-ID: <20261025154745.2734805-1-tristmd@gmail.com> (raw)
In-Reply-To: <20261024205850.4051206-1-tristmd@gmail.com>
From: Tristan Madani <tristan@talencesecurity.com>
The QuickI2C and QuickSPI ACPI helper functions copy firmware-returned
buffers into caller-provided structures without verifying the buffer
length. Malformed or short ACPI tables could cause an out-of-bounds
read from the firmware buffer, and oversized responses could overflow
the destination struct.
Additionally, quicki2c_acpi_get_dsd_property() does not validate the
returned ACPI object type before accessing buffer fields, and does not
free the allocated ACPI buffer on the success path, leaking memory on
every call.
Add a buf_size parameter to the ACPI helper functions and reject
responses larger than the destination buffer. Accept shorter responses
to handle firmware that returns buffers without struct tail padding
(e.g. ISUB tables returning 177 or 145 bytes for a 184-byte struct).
Zero-initialize caller stack structures so fields beyond the firmware
response length are deterministic. Add the missing type check and
kfree to the DSD helper.
Fixes: 5282e45ccbfa ("HID: intel-thc-hid: intel-quicki2c: Add THC QuickI2C ACPI interfaces")
Fixes: 7cb06f08ef13 ("HID: intel-thc-hid: intel-quickspi: Add THC QuickSPI ACPI interfaces")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
Changes in v2:
- Use overflow-only check (>) instead of exact-match (!=) so firmware
returning shorter buffers (e.g. 177 or 145 bytes for 184-byte ISUB
struct, or 1 byte for 4-byte SPI opcode) is accepted
- Zero-initialize caller stack structures for deterministic behavior
with short firmware responses
- Add missing type validation and kfree in DSD helper
- Add braces per coding style
.../intel-quicki2c/pci-quicki2c.c | 40 ++++++++++++++-----
.../intel-quickspi/pci-quickspi.c | 36 ++++++++++-------
2 files changed, 51 insertions(+), 25 deletions(-)
diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
index 7a913f717efaad..dd21864d16b05b 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
@@ -62,7 +62,8 @@ static const struct acpi_gpio_mapping quicki2c_gpios[] = {
* Return: 0 if success or ENODEV on failure.
*/
static int quicki2c_acpi_get_dsm_property(struct acpi_device *adev, const guid_t *guid,
- u64 rev, u64 func, acpi_object_type type, void *prop_buf)
+ u64 rev, u64 func, acpi_object_type type,
+ void *prop_buf, size_t buf_size)
{
acpi_handle handle = acpi_device_handle(adev);
union acpi_object *obj;
@@ -75,10 +76,15 @@ static int quicki2c_acpi_get_dsm_property(struct acpi_device *adev, const guid_t
return -ENODEV;
}
- if (type == ACPI_TYPE_INTEGER)
+ if (type == ACPI_TYPE_INTEGER) {
*(u32 *)prop_buf = (u32)obj->integer.value;
- else if (type == ACPI_TYPE_BUFFER)
+ } else if (type == ACPI_TYPE_BUFFER) {
+ if (buf_size > 0 && obj->buffer.length > buf_size) {
+ ACPI_FREE(obj);
+ return -ENOSPC;
+ }
memcpy(prop_buf, obj->buffer.pointer, obj->buffer.length);
+ }
ACPI_FREE(obj);
@@ -97,7 +103,8 @@ static int quicki2c_acpi_get_dsm_property(struct acpi_device *adev, const guid_t
* Return: 0 if success or ENODEV on failed.
*/
static int quicki2c_acpi_get_dsd_property(struct acpi_device *adev, acpi_string dsd_method_name,
- acpi_object_type type, void *prop_buf)
+ acpi_object_type type, void *prop_buf,
+ size_t buf_size)
{
acpi_handle handle = acpi_device_handle(adev);
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -113,7 +120,16 @@ static int quicki2c_acpi_get_dsd_property(struct acpi_device *adev, acpi_string
ret_obj = buffer.pointer;
+ if (ret_obj->type != type) {
+ kfree(buffer.pointer);
+ return -EINVAL;
+ }
+ if (buf_size > 0 && ret_obj->buffer.length > buf_size) {
+ kfree(buffer.pointer);
+ return -ENOSPC;
+ }
memcpy(prop_buf, ret_obj->buffer.pointer, ret_obj->buffer.length);
+ kfree(buffer.pointer);
return 0;
}
@@ -129,8 +145,8 @@ static int quicki2c_acpi_get_dsd_property(struct acpi_device *adev, acpi_string
static int quicki2c_get_acpi_resources(struct quicki2c_device *qcdev)
{
struct acpi_device *adev = ACPI_COMPANION(qcdev->dev);
- struct quicki2c_subip_acpi_parameter i2c_param;
- struct quicki2c_subip_acpi_config i2c_config;
+ struct quicki2c_subip_acpi_parameter i2c_param = {};
+ struct quicki2c_subip_acpi_config i2c_config = {};
u32 hid_desc_addr;
int ret = -EINVAL;
@@ -145,7 +161,7 @@ static int quicki2c_get_acpi_resources(struct quicki2c_device *qcdev)
QUICKI2C_ACPI_REVISION_NUM,
QUICKI2C_ACPI_FUNC_NUM_HID_DESC_ADDR,
ACPI_TYPE_INTEGER,
- &hid_desc_addr);
+ &hid_desc_addr, 0);
if (ret)
return ret;
@@ -155,7 +171,7 @@ static int quicki2c_get_acpi_resources(struct quicki2c_device *qcdev)
QUICKI2C_ACPI_REVISION_NUM,
QUICKI2C_ACPI_FUNC_NUM_ACTIVE_LTR_VAL,
ACPI_TYPE_INTEGER,
- &qcdev->active_ltr_val);
+ &qcdev->active_ltr_val, 0);
if (ret)
return ret;
@@ -163,12 +179,13 @@ static int quicki2c_get_acpi_resources(struct quicki2c_device *qcdev)
QUICKI2C_ACPI_REVISION_NUM,
QUICKI2C_ACPI_FUNC_NUM_LP_LTR_VAL,
ACPI_TYPE_INTEGER,
- &qcdev->low_power_ltr_val);
+ &qcdev->low_power_ltr_val, 0);
if (ret)
return ret;
ret = quicki2c_acpi_get_dsd_property(adev, QUICKI2C_ACPI_METHOD_NAME_ICRS,
- ACPI_TYPE_BUFFER, &i2c_param);
+ ACPI_TYPE_BUFFER, &i2c_param,
+ sizeof(i2c_param));
if (ret)
return ret;
@@ -180,7 +197,8 @@ static int quicki2c_get_acpi_resources(struct quicki2c_device *qcdev)
qcdev->i2c_config.target_addr = i2c_param.device_address;
ret = quicki2c_acpi_get_dsd_property(adev, QUICKI2C_ACPI_METHOD_NAME_ISUB,
- ACPI_TYPE_BUFFER, &i2c_config);
+ ACPI_TYPE_BUFFER, &i2c_config,
+ sizeof(i2c_config));
if (ret)
return ret;
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
index 53023e30cf8d47..e4331ba4e503d9 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -81,7 +81,8 @@ static const struct acpi_gpio_mapping quickspi_gpios[] = {
* Return: 0 if successful or ENODEV on failed.
*/
static int thc_acpi_get_property(struct acpi_device *adev, const guid_t *guid,
- u64 rev, u64 func, acpi_object_type type, void *prop_buf)
+ u64 rev, u64 func, acpi_object_type type,
+ void *prop_buf, size_t buf_size)
{
acpi_handle handle = acpi_device_handle(adev);
union acpi_object *obj;
@@ -94,10 +95,15 @@ static int thc_acpi_get_property(struct acpi_device *adev, const guid_t *guid,
return -ENODEV;
}
- if (type == ACPI_TYPE_INTEGER)
+ if (type == ACPI_TYPE_INTEGER) {
*(u32 *)prop_buf = (u32)obj->integer.value;
- else if (type == ACPI_TYPE_BUFFER)
+ } else if (type == ACPI_TYPE_BUFFER) {
+ if (buf_size > 0 && obj->buffer.length > buf_size) {
+ ACPI_FREE(obj);
+ return -ENOSPC;
+ }
memcpy(prop_buf, obj->buffer.pointer, obj->buffer.length);
+ }
ACPI_FREE(obj);
@@ -129,7 +135,7 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_INPUT_REP_HDR_ADDR,
ACPI_TYPE_INTEGER,
- &qsdev->input_report_hdr_addr);
+ &qsdev->input_report_hdr_addr, 0);
if (ret)
return ret;
@@ -137,7 +143,7 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_INPUT_REP_BDY_ADDR,
ACPI_TYPE_INTEGER,
- &qsdev->input_report_bdy_addr);
+ &qsdev->input_report_bdy_addr, 0);
if (ret)
return ret;
@@ -145,7 +151,7 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_OUTPUT_REP_ADDR,
ACPI_TYPE_INTEGER,
- &qsdev->output_report_addr);
+ &qsdev->output_report_addr, 0);
if (ret)
return ret;
@@ -153,7 +159,8 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_READ_OPCODE,
ACPI_TYPE_BUFFER,
- &qsdev->spi_read_opcode);
+ &qsdev->spi_read_opcode,
+ sizeof(qsdev->spi_read_opcode));
if (ret)
return ret;
@@ -161,7 +168,8 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_WRITE_OPCODE,
ACPI_TYPE_BUFFER,
- &qsdev->spi_write_opcode);
+ &qsdev->spi_write_opcode,
+ sizeof(qsdev->spi_write_opcode));
if (ret)
return ret;
@@ -169,7 +177,7 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_IO_MODE,
ACPI_TYPE_INTEGER,
- &qsdev->spi_read_io_mode);
+ &qsdev->spi_read_io_mode, 0);
if (ret)
return ret;
@@ -184,7 +192,7 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_CONNECTION_SPEED,
ACPI_TYPE_INTEGER,
- &qsdev->spi_freq_val);
+ &qsdev->spi_freq_val, 0);
if (ret)
return ret;
@@ -192,7 +200,7 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_LIMIT_PACKET_SIZE,
ACPI_TYPE_INTEGER,
- &qsdev->limit_packet_size);
+ &qsdev->limit_packet_size, 0);
if (ret)
return ret;
@@ -205,7 +213,7 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_PERFORMANCE_LIMIT,
ACPI_TYPE_INTEGER,
- &qsdev->performance_limit);
+ &qsdev->performance_limit, 0);
if (ret)
return ret;
@@ -215,7 +223,7 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_ACTIVE_LTR,
ACPI_TYPE_INTEGER,
- &qsdev->active_ltr_val);
+ &qsdev->active_ltr_val, 0);
if (ret)
return ret;
@@ -223,7 +231,7 @@ static int quickspi_get_acpi_resources(struct quickspi_device *qsdev)
ACPI_QUICKSPI_REVISION_NUM,
ACPI_QUICKSPI_FUNC_NUM_LP_LTR,
ACPI_TYPE_INTEGER,
- &qsdev->low_power_ltr_val);
+ &qsdev->low_power_ltr_val, 0);
if (ret)
return ret;
--
2.47.3
next prev parent reply other threads:[~2026-10-25 15:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-24 20:58 [PATCH] HID: intel-thc-hid: validate ACPI buffer length before use Tristan Madani
2026-10-25 10:49 ` kernel test robot
2026-10-25 15:47 ` Tristan Madani [this message]
2026-10-26 2:02 ` [PATCH v2] HID: intel-thc-hid: validate ACPI buffer length before memcpy Xu, Even
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=20261025154745.2734805-1-tristmd@gmail.com \
--to=tristmd@gmail.com \
--cc=bentiss@kernel.org \
--cc=even.xu@intel.com \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mpearson-lenovo@squebb.ca \
--cc=srinivas.pandruvada@linux.intel.com \
--cc=stable@vger.kernel.org \
--cc=tristan@talencesecurity.com \
--cc=xinpeng.sun@intel.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®