Thread (16 messages) flat view 16 messages, 3 authors, 13d ago

Re: [PATCH v8] HID: logitech-hidpp: Add support for HID++ Multi-Platform feature (0x4531)

From: sashiko-bot@kernel.org
Date: 2026-08-26 08:08:07

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing cancellation of `reset_hi_res_work` in the `hidpp_probe` error path, leading to a potential Use-After-Free.
--

commit 452cf6e851cc2adb7ff83d84efb83964e63b45a1
Author: Baraa Atta (Dev Exalt) [off-list ref]

HID: logitech-hidpp: Add support for HID++ Multi-Platform feature (0x4531)

This commit adds support for the HID++ Multi-Platform feature, introducing a
per-device sysfs attribute to allow selecting the target platform. It detects
the feature, validates requests, and alters device behavior accordingly.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 90b0184df777f..1cb78096f31c5 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -4557,6 +5154,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 hid_hw_open_fail:
 	hid_hw_stop(hdev);
 hid_hw_start_fail:
+	hidpp_multiplatform_cleanup(hidpp);
 	sysfs_remove_group(&hdev->dev.kobj, &ps_attribute_group);
 	cancel_work_sync(&hidpp->work);
 	mutex_destroy(&hidpp->send_mutex);
 	return ret;
 }
[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
vulnerability in this error path.

Could this leave a pending work item running on freed memory if a device
sends a raw hardware event during the probe sequence?

The reset_hi_res_work is initialized earlier in hidpp_probe() and
could be scheduled immediately after hid_device_io_start() is called.
If a subsequent initialization step like hid_connect() fails, we end up
in this error path which cancels hidpp->work, but skips canceling
hidpp->reset_hi_res_work.

The core devres mechanism will then free the hidpp structure, potentially
allowing the pending work to execute on freed memory.

Should cancel_work_sync(&hidpp->reset_hi_res_work); be added to this
cleanup path, similarly to how it is correctly handled in hidpp_remove()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826075758.47287-1-exalt.dev.team@gmail.com?part=1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help