[PATCH 1/4] HID: lamparray: read attribute reports synchronously
From: Cristian Mazzotta <hidden>
Date: 2026-09-09 16:53:13
Also in:
lkml
Subsystem:
hid core layer, the rest · Maintainers:
Jiri Kosina, Benjamin Tissoires, Linus Torvalds
lamparray_read_attributes_report() and lamparray_get_lamp_attributes()
use hid_hw_request(HID_REQ_GET_REPORT) followed by hid_hw_wait(), then
read the results out of field->value[]. hid_hw_request() is asynchronous
and hid_hw_wait() only clears the output queue, so the values are still
read before the transfer has completed.
On an Acer Predator PT14-52T (USB keyboard 05AF:767A), this returns zero
for LampCount, and then zero for red, green, blue, and intensity counts.
The last one is evil: max_brightness is passed to
led_mc_calc_color_components() as a divisor during
lamparray_register_led(), resulting in a divide by zero during probe:
Oops: divide error: 0000 [#1] SMP NOPTI
RIP: 0010:led_mc_calc_color_components+0x58/0x70
Call Trace:
lamparray_register_led+0x119/0x1e0
lamparray_register+0x502/0x820
hid_generic_probe+0x5e/0xc0
The fault kills the kworker running hub_event() while it holds the USB
and HID device locks, which stalls further probing on that bus.
Use hid_hw_raw_request() with a hid_report_len()-sized buffer and hand
the result to hid_report_raw_event() so the HID core parses it into
field->value[] before the values are read.
Validate the level counts after reading them and fail with -EINVAL if
any is zero, so a device reporting no levels cannot reach
led_mc_calc_color_components() at all. The four level fields are also
required to share one report, since a single GET_REPORT is used to
fetch them.
Signed-off-by: Cristian Mazzotta <redacted>
---
drivers/hid/hid-lamparray.c | 73 ++++++++++++++++++++++++++++++-------
1 file changed, 59 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-lamparray.c b/drivers/hid/hid-lamparray.c
index 9a438aa2d305..f169929aecd6 100644
--- a/drivers/hid/hid-lamparray.c
+++ b/drivers/hid/hid-lamparray.c@@ -164,6 +164,9 @@ static int lamparray_read_attributes_report(struct lamparray_device *ldev) { struct hid_device *hdev = ldev->hdev; struct hid_report *report; + int ret; + u8 *buf; + size_t len; if (!ldev->lamp_count.field) { hid_dbg(hdev, "No LampCount field found\n");
@@ -182,25 +185,37 @@ static int lamparray_read_attributes_report(struct lamparray_device *ldev) return -ENODEV; } + len = hid_report_len(report); + buf = kmalloc(len, GFP_KERNEL); + if (!buf) + return -ENOMEM; + mutex_lock(&ldev->dev_lock); /* Update values */ - hid_hw_request(hdev, report, HID_REQ_GET_REPORT); - hid_hw_wait(hdev); + ret = hid_hw_raw_request(hdev, report->id, buf, len, + HID_FEATURE_REPORT, HID_REQ_GET_REPORT); + if (ret < 0) { + hid_dbg(hdev, "Failed to get LampCount value from device: %d\n", ret); + goto out; + } - ldev->lamp_count_value = get_field_value(&ldev->lamp_count); + hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf, len, ret, 0); - if (ldev->lamp_count_value == 0) { - mutex_unlock(&ldev->dev_lock); - hid_dbg(hdev, "LampCount is %d (invalid)\n", ldev->lamp_count_value); - return -EINVAL; + ldev->lamp_count_value = get_field_value(&ldev->lamp_count); + if (!ldev->lamp_count_value) { + hid_dbg(hdev, "LampCount is 0 (invalid)\n"); + ret = -EINVAL; + goto out; } ldev->lamparray_kind_value = get_field_value(&ldev->lamparray_kind); + ret = 0; +out: mutex_unlock(&ldev->dev_lock); - - return 0; + kfree(buf); + return ret; } static int lamparray_parse_update_report(struct lamparray_device *ldev)
@@ -371,28 +386,58 @@ static int lamparray_get_lamp_attributes(struct lamparray_device *ldev) { struct hid_device *hdev = ldev->hdev; struct hid_report *report; + int ret; + u8 *buf; + size_t len; if (!lamparray_color_fields_is_complete(&ldev->color_levels)) return -ENODEV; /* - * Get value of any lamp. + * All four fields must share the same report since the + * attributes are fetched with a single GET_REPORT below. */ report = ldev->color_levels.red.field->report; + if (!report || + ldev->color_levels.green.field->report != report || + ldev->color_levels.blue.field->report != report || + ldev->color_levels.intensity.field->report != report) + return -ENODEV; + + len = hid_report_len(report); + buf = kmalloc(len, GFP_KERNEL); + if (!buf) + return -ENOMEM; mutex_lock(&ldev->dev_lock); - hid_hw_request(hdev, report, HID_REQ_GET_REPORT); - hid_hw_wait(hdev); + /* Update values */ + ret = hid_hw_raw_request(hdev, report->id, buf, len, + HID_FEATURE_REPORT, HID_REQ_GET_REPORT); + if (ret < 0) { + hid_dbg(hdev, "Failed to read LampAttributesResponseReport: %d\n", ret); + goto out; + } + + hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf, len, ret, 0); ldev->max_r = get_field_value_as_u8(&ldev->color_levels.red); ldev->max_g = get_field_value_as_u8(&ldev->color_levels.green); ldev->max_b = get_field_value_as_u8(&ldev->color_levels.blue); ldev->max_brightness = get_field_value_as_u8(&ldev->color_levels.intensity); - mutex_unlock(&ldev->dev_lock); + if (!ldev->max_r || !ldev->max_g || !ldev->max_b || !ldev->max_brightness) { + hid_dbg(hdev, "LampArray device has no color levels\n"); + ret = -EINVAL; + goto out; + } - return 0; + ret = 0; + +out: + mutex_unlock(&ldev->dev_lock); + kfree(buf); + return ret; } /* Helper functions */
--
2.55.0