Re: [PATCH] HID: i2c-hid: add quirk to send initial output reports for Acer SFA14-I31
From: sashiko-bot@kernel.org
Date: 2026-10-03 01:33:41
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] hid: i2c-hid: silent quirk failure due to dependency on synchronous driver binding - [High] hid: i2c-hid: unprotected report_list traversal causes data race and UAF -- commit 8bcb0b187653aa75d332fb5ab3f4c9d75928faf8 Author: DMS [off-list ref] HID: i2c-hid: add quirk to send initial output reports for Acer SFA14-I31 The internal keyboard of the Acer Swift Air 14 (SFA14-I31, ACPI 1025174B, HID 049f:5343) does not send any input reports after probe. It stays silent, and its GPIO interrupt never fires, until the host has written an output report to it at least once. Add I2C_HID_QUIRK_INIT_OUTPUT_REPORTS, which sends every output report once with its initial field values right after the HID device has been added, and enable it for this keyboard.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index a8a9854854f52..f546710925e64 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1079,6 +1086,32 @@ static int __i2c_hid_core_probe(struct i2c_hid *ihid) return 0; } +/* + * The input core only writes LED state when it changes (and on resume), so + * a device that waits for an output report never gets one at boot. Send + * every output report once with its initial field values. + */ +static void i2c_hid_send_initial_output_reports(struct hid_device *hid) +{ + struct hid_report_enum *report_enum = &hid->report_enum[HID_OUTPUT_REPORT]; + struct hid_report *report; + u8 *buf; + int ret; + + list_for_each_entry(report, &report_enum->report_list, list) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [Severity: High] Does this unsynchronized iteration risk a data race or use-after-free? Because hid_add_device() has already exposed the device to the driver core and userspace, the high-level driver could be binding asynchronously and mutating the list via hid_register_report(), or unbinding and freeing the reports concurrently via hid_close_report(). [ ... ]
quoted hunk ↗ jump to hunk
@@ -1099,6 +1132,9 @@ static int i2c_hid_core_register_hid(struct i2c_hid *ihid) if (ihid->quirks & I2C_HID_QUIRK_RE_POWER_ON) ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON); + if (ihid->quirks & I2C_HID_QUIRK_INIT_OUTPUT_REPORTS) + i2c_hid_send_initial_output_reports(hid);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [Severity: High] Will this reliably send the initial reports? Because this executes immediately after hid_add_device() registers the device, reports are not parsed and added to report_list yet. The list is only populated when the high-level driver (e.g. hid-generic) probes and calls hid_parse(). If the high-level driver binds asynchronously or is loaded as a module after i2c-hid, i2c_hid_send_initial_output_reports() will iterate over an empty list and silently do nothing, leaving the keyboard unresponsive.
quoted hunk ↗ jump to hunk
+ return ret; }
-- Sashiko AI review · https://sashiko.dev/#/patchset/20261002-swift-sfa14-kbd-v1-1-ad312bca1076@gmail.com?part=1