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

Pre-existing issues:
- [High] Dangling pointer `hidpp->input` leads to Use-After-Free if `input_register_device()` fails.
- [High] Unbounded 64-byte structure copy in `hidpp_raw_hidpp_event` causes a Kernel Heap Out-of-Bounds Read.
--

commit 13dc21807e8eb6f76d9029e89353e4bd2e6aa1c3
Author: Elliot Douglas [off-list ref]
Subject: HID: logitech-hidpp: add HID++ 2.0 reprogrammable button support

Some Logitech HID++ 2.0 mice can report diverted reprogrammable controls
through HID++ feature 0x1b04, SpecialKeysMseButtons / REPROG_CONTROLS_V4,
instead of the normal HID mouse report.

Link: https://lekensteyn.nl/files/logitech/x1b04_specialkeysmsebuttons.html

[ ... ]

[ ... ]

+static int hidpp20_reprog_controls_raw_event(struct hidpp_device *hidpp,
+					     u8 *data, int size)
+{
+	const struct hidpp_reprog_control_mapping *mapping;
+	struct hidpp_report *report = (struct hidpp_report *)data;
+	u16 controls[4];
+	bool pressed;
+	unsigned int i, j;
+
+	if (!(hidpp->quirks & HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS) ||
+	    !hidpp->input ||

[Severity: High]
This is a pre-existing issue, but could this lead to a use-after-free?

Looking at hidpp_connect_event(), if input_register_device() fails, it
cleans up by calling input_free_device() but leaves the hidpp->input 
pointer dangling:

    ret = input_register_device(input);
    if (ret) {
            input_free_device(input);
            return;
    }

Because hid_device_io_start() has already enabled incoming packets before
this point, could a newly arriving event trigger this new
hidpp20_reprog_controls_raw_event() handler, pass the !hidpp->input check,
and dereference the freed pointer?

+	    !hidpp->reprog_controls ||
+	    hidpp->reprog_controls_feature_index == 0xff)
+		return 0;

[ ... ]

[Severity: High]
This is a pre-existing issue, but does hidpp_raw_hidpp_event() perform an
out-of-bounds read on the incoming data buffer?

Earlier in this function, if the packet matches a pending question, it does
a direct structure assignment:

    if (hidpp_match_answer(question, report) ||
            hidpp_match_error(question, report)) {
        *answer = *report;

Since struct hidpp_report is a 64-byte packed structure, won't this pointer
assignment perform a fixed 64-byte copy regardless of the actual size of the
received buffer (which could be as small as 7 bytes for a 
REPORT_ID_HIDPP_SHORT packet)? 

Could this copy adjacent heap memory into the answer buffer, potentially 
leaking it when the data is parsed later?

 	if (hidpp->quirks & HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS) {
 		ret = hidpp10_consumer_keys_raw_event(hidpp, data, size);
 		if (ret != 0)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783094954.git.edouglas7358@gmail.com?part=1