[PATCH v3 0/2] HID: logitech-hidpp: fix Signature M650 side button timing

[PATCH v3 0/2] HID: logitech-hidpp: fix Signature M650 side button timing

[Elliot Douglas]

The Logitech Signature M650 over Bluetooth exposes its side buttons in the
normal mouse report, but the reported BTN_SIDE/BTN_EXTRA events are short
click-like events emitted around button release rather than physical
press/release events with the real hold duration. The device appears to reserve
the held side-button state for a built-in gesture mode: holding a side button
more than approximately 2 seconds, or holding it while using the wheel for
horizontal scrolling, can mean the normal mouse report never emits a usable
side-button press at all. That makes the buttons unusable for standard Linux
hold actions such as push-to-talk, drag modifiers, or remapping rules that
depend on key-up timing.

When HID++ 2.0 feature 0x1b04, SpecialKeysMseButtons /
REPROG_CONTROLS_V4, temporarily diverts the same controls, the device sends
diverted-control notifications with real press and release timing. This series
adds quirk-gated support for those notifications and enables it for the
Bluetooth Signature M650.

Before enabling diversion, the driver verifies that each mapped control is
present in the device's HID++ control table and is advertised as a divertable
mouse control.

The kernel only programs temporary diversion when the device connects. It
does not continuously force the setting. A userspace HID++ tool such as
Solaar can still issue HID++ commands through hidraw; if Solaar changes the
reporting mode for the same controls afterwards, the last writer wins. That
means Solaar can still take over those controls for custom actions, but the
kernel will no longer receive the diverted button notifications for normal
evdev reporting until the kernel diverts the controls again, for example after
reconnect. While the controls remain diverted, hidraw clients should still
receive the raw reports and the kernel reports the matching evdev state.

The diverted M650 controls are reported as BTN_BACK and BTN_FORWARD. Logitech's
Signature M650 getting-started page labels these physical controls as
Back/Forward buttons and describes their default page-navigation behavior:
https://support.logi.com/hc/en-nz/articles/4414473810583-Getting-Started-Signature-M650

The reprogrammable-control support is per-product and parses the full HID++
divertedButtonsEvent pressed-control list, so it can support devices with more
buttons without relying on a single last-control release heuristic. Only the
Signature M650 opts in for now. Other Logitech devices should only be enabled
after their HID++ control IDs and divertedButtonsEvent behavior are captured
and verified.

There is evidence that this is not unique to the M650. A prior MX Anywhere 3
patch used the same HID++ feature to fix thumb buttons that only activated on
release, and Logitech documents side-button + wheel horizontal scrolling for
both the MX Anywhere 3/3S and Signature M650. Solaar's device reports and rules
documentation also show HID++ divertable back/forward controls on MX Master 3
and MX Master 3S class devices. This series remains conservative and only
enables the device tested here.

Tested with a Logitech Signature M650 L over Bluetooth, HID ID
0005:046D:B02A. Baseline evtest showed short release-time BTN_SIDE/BTN_EXTRA
events. Earlier local testing of the same HID++ diversion path showed real
hold-duration press/release events, including holds longer than 4 seconds for
both buttons.

Changes in v3:
- Move the M650 product ID define to drivers/hid/hid-ids.h.
- Clarify the M650 long-hold threshold in the patch text.
- Add Bastien's Reviewed-by tag to patch 2.

Changes in v2:
- Replace the profile/count wrapper with NULL-terminated mapping arrays.
- Cache the selected reprogrammable-control mapping in struct hidpp_device.
- Add a named constant for the M650 Bluetooth product ID.
- Use common Back/Forward names for the 0x0053/0x0056 HID++ control IDs.

v2: https://lore.kernel.org/r/cover.1783094954.git.edouglas7358@gmail.com
v1: https://lore.kernel.org/r/20260613175109.44365-1-edouglas7358@gmail.com

Elliot Douglas (2):
  HID: logitech-hidpp: add HID++ 2.0 reprogrammable button support
  HID: logitech-hidpp: enable reprogrammable buttons on Signature M650

 drivers/hid/hid-ids.h            |   1 +
 drivers/hid/hid-logitech-hidpp.c | 223 ++++++++++++++++++++++++++++++-
 2 files changed, 223 insertions(+), 1 deletion(-)


base-commit: f0866517be9345d8245d32b722574b8aecccb348
-- 
2.55.0

[PATCH v3 1/2] HID: logitech-hidpp: add HID++ 2.0 reprogrammable button support

[Elliot Douglas]

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.

Add a quirk-gated event path for those controls. The handler temporarily
diverts verified per-product controls, parses divertedButtonsEvent as the
current pressed-control list, and reports the corresponding evdev key state
for every mapped control.

Keep the control mappings in per-product arrays so adding support for
another mouse does not change the evdev capabilities advertised by
already-supported devices.

Documentation for feature 0x1b04 describes divertedButtonsEvent as a list
of currently pressed diverted buttons, which is the event format handled
here.

Link: https://lekensteyn.nl/files/logitech/x1b04_specialkeysmsebuttons.html
Signed-off-by: Elliot Douglas <edouglas7358@gmail.com>
---
 drivers/hid/hid-logitech-hidpp.c | 205 +++++++++++++++++++++++++++++++
 1 file changed, 205 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 70ba1a5e40d8..f9189e14fb78 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -76,6 +76,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_HI_RES_SCROLL_1P0		BIT(28)
 #define HIDPP_QUIRK_WIRELESS_STATUS		BIT(29)
 #define HIDPP_QUIRK_RESET_HI_RES_SCROLL		BIT(30)
+#define HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS	BIT(31)
 
 /* These are just aliases for now */
 #define HIDPP_QUIRK_KBD_SCROLL_WHEEL HIDPP_QUIRK_HIDPP_WHEELS
@@ -178,6 +179,8 @@ struct hidpp_scroll_counter {
 	unsigned long long last_time;
 };
 
+struct hidpp_reprog_control_mapping;
+
 struct hidpp_device {
 	struct hid_device *hid_dev;
 	struct input_dev *input;
@@ -205,6 +208,8 @@ struct hidpp_device {
 	struct hidpp_scroll_counter vertical_wheel_counter;
 
 	u8 wireless_feature_index;
+	u8 reprog_controls_feature_index;
+	const struct hidpp_reprog_control_mapping *reprog_controls;
 
 	int hires_wheel_multiplier;
 	u8 hires_wheel_feature_index;
@@ -3601,6 +3606,195 @@ static int hidpp10_extra_mouse_buttons_raw_event(struct hidpp_device *hidpp,
 	return 1;
 }
 
+/* -------------------------------------------------------------------------- */
+/* HID++2.0 reprogrammable controls                                           */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_REPROG_CONTROLS_V4			0x1b04
+
+#define HIDPP_REPROG_CONTROLS_GET_COUNT			0x00
+#define HIDPP_REPROG_CONTROLS_GET_CID_INFO		0x10
+#define HIDPP_REPROG_CONTROLS_SET_CONTROL_REPORTING	0x30
+
+#define HIDPP_REPROG_CONTROLS_FLAG_MOUSE		BIT(0)
+#define HIDPP_REPROG_CONTROLS_FLAG_DIVERT		BIT(5)
+
+#define HIDPP_REPROG_CONTROLS_TEMPORARY_DIVERTED	BIT(0)
+#define HIDPP_REPROG_CONTROLS_CHANGE_TEMPORARY_DIVERT	BIT(1)
+
+#define HIDPP_REPROG_CONTROLS_EVENT_DIVERTED		0x00
+
+struct hidpp_reprog_control_mapping {
+	u16 control;
+	u16 code;
+};
+
+static const struct hidpp_reprog_control_mapping *
+hidpp20_reprog_controls_get_mappings(struct hidpp_device *hidpp)
+{
+	return NULL;
+}
+
+static int hidpp20_reprog_controls_get_count(struct hidpp_device *hidpp)
+{
+	struct hidpp_report response;
+	u8 feature_index = hidpp->reprog_controls_feature_index;
+	u8 cmd = HIDPP_REPROG_CONTROLS_GET_COUNT;
+	int ret;
+
+	ret = hidpp_send_fap_command_sync(hidpp, feature_index, cmd, NULL, 0,
+					  &response);
+	if (ret > 0)
+		return -EPROTO;
+	if (ret)
+		return ret;
+
+	return response.fap.params[0];
+}
+
+static int hidpp20_reprog_controls_get_cid_info(struct hidpp_device *hidpp,
+						u8 index, u16 *control,
+						u8 *flags)
+{
+	struct hidpp_report response;
+	u8 feature_index = hidpp->reprog_controls_feature_index;
+	u8 cmd = HIDPP_REPROG_CONTROLS_GET_CID_INFO;
+	int ret;
+
+	ret = hidpp_send_fap_command_sync(hidpp, feature_index, cmd, &index,
+					  sizeof(index), &response);
+	if (ret > 0)
+		return -EPROTO;
+	if (ret)
+		return ret;
+
+	*control = get_unaligned_be16(&response.fap.params[0]);
+	*flags = response.fap.params[4];
+
+	return 0;
+}
+
+static bool hidpp20_reprog_controls_find_control(struct hidpp_device *hidpp,
+						 u16 control)
+{
+	int count, ret;
+	u16 cid;
+	u8 flags;
+	int i;
+
+	count = hidpp20_reprog_controls_get_count(hidpp);
+	if (count < 0)
+		return false;
+
+	for (i = 0; i < count; i++) {
+		ret = hidpp20_reprog_controls_get_cid_info(hidpp, i, &cid,
+							   &flags);
+		if (ret)
+			return false;
+
+		if (cid == control)
+			return (flags & HIDPP_REPROG_CONTROLS_FLAG_MOUSE) &&
+			       (flags & HIDPP_REPROG_CONTROLS_FLAG_DIVERT);
+	}
+
+	return false;
+}
+
+static int hidpp20_reprog_controls_set_control_reporting(struct hidpp_device *hidpp,
+							 u16 control, u8 flags)
+{
+	struct hidpp_report response;
+	u8 params[5];
+
+	put_unaligned_be16(control, &params[0]);
+	params[2] = flags;
+	put_unaligned_be16(control, &params[3]);
+
+	return hidpp_send_fap_command_sync(hidpp,
+					   hidpp->reprog_controls_feature_index,
+					   HIDPP_REPROG_CONTROLS_SET_CONTROL_REPORTING,
+					   params, sizeof(params), &response);
+}
+
+static void hidpp20_reprog_controls_connect(struct hidpp_device *hidpp)
+{
+	const struct hidpp_reprog_control_mapping *mapping;
+	u8 flags = HIDPP_REPROG_CONTROLS_TEMPORARY_DIVERTED |
+		   HIDPP_REPROG_CONTROLS_CHANGE_TEMPORARY_DIVERT;
+
+	if (!(hidpp->quirks & HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS))
+		return;
+
+	if (!hidpp->reprog_controls)
+		return;
+
+	if (hidpp_root_get_feature(hidpp, HIDPP_PAGE_REPROG_CONTROLS_V4,
+				   &hidpp->reprog_controls_feature_index))
+		return;
+
+	for (mapping = hidpp->reprog_controls; mapping->control; mapping++) {
+		if (!hidpp20_reprog_controls_find_control(hidpp, mapping->control))
+			continue;
+
+		hidpp20_reprog_controls_set_control_reporting(hidpp,
+							      mapping->control,
+							      flags);
+	}
+}
+
+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 ||
+	    !hidpp->reprog_controls ||
+	    hidpp->reprog_controls_feature_index == 0xff)
+		return 0;
+
+	if (size < HIDPP_REPORT_LONG_LENGTH ||
+	    report->fap.feature_index != hidpp->reprog_controls_feature_index ||
+	    report->fap.funcindex_clientid != HIDPP_REPROG_CONTROLS_EVENT_DIVERTED)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(controls); i++)
+		controls[i] = get_unaligned_be16(&report->fap.params[i * 2]);
+
+	for (mapping = hidpp->reprog_controls; mapping->control; mapping++) {
+		pressed = false;
+
+		for (j = 0; j < ARRAY_SIZE(controls); j++) {
+			if (controls[j] == mapping->control) {
+				pressed = true;
+				break;
+			}
+		}
+
+		input_report_key(hidpp->input, mapping->code, pressed);
+	}
+
+	input_sync(hidpp->input);
+
+	return 1;
+}
+
+static void hidpp20_reprog_controls_populate_input(struct hidpp_device *hidpp,
+						   struct input_dev *input_dev)
+{
+	const struct hidpp_reprog_control_mapping *mapping;
+
+	if (!hidpp->reprog_controls)
+		return;
+
+	for (mapping = hidpp->reprog_controls; mapping->control; mapping++)
+		input_set_capability(input_dev, EV_KEY, mapping->code);
+}
+
 static void hidpp10_extra_mouse_buttons_populate_input(
 			struct hidpp_device *hidpp, struct input_dev *input_dev)
 {
@@ -3859,6 +4053,9 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
 
 	if (hidpp->quirks & HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS)
 		hidpp10_extra_mouse_buttons_populate_input(hidpp, input);
+
+	if (hidpp->quirks & HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS)
+		hidpp20_reprog_controls_populate_input(hidpp, input);
 }
 
 static int hidpp_input_configured(struct hid_device *hdev,
@@ -3971,6 +4168,10 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 			return ret;
 	}
 
+	ret = hidpp20_reprog_controls_raw_event(hidpp, data, size);
+	if (ret != 0)
+		return ret;
+
 	if (hidpp->quirks & HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS) {
 		ret = hidpp10_consumer_keys_raw_event(hidpp, data, size);
 		if (ret != 0)
@@ -4264,6 +4465,8 @@ static void hidpp_connect_event(struct work_struct *work)
 			return;
 	}
 
+	hidpp20_reprog_controls_connect(hidpp);
+
 	if (hidpp->quirks & HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS) {
 		ret = hidpp10_consumer_keys_connect(hidpp);
 		if (ret)
@@ -4436,6 +4639,8 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	hidpp->hid_dev = hdev;
 	hidpp->name = hdev->name;
 	hidpp->quirks = id->driver_data;
+	hidpp->reprog_controls_feature_index = 0xff;
+	hidpp->reprog_controls = hidpp20_reprog_controls_get_mappings(hidpp);
 	hid_set_drvdata(hdev, hidpp);
 
 	ret = hid_parse(hdev);
-- 
2.55.0

[PATCH v3 2/2] HID: logitech-hidpp: enable reprogrammable buttons on Signature M650

[Elliot Douglas]

The Bluetooth Signature M650 exposes its side buttons through the normal
mouse report, but the observed events are short click-like events emitted
around release rather than physical press/release state.

The device appears to use the held side-button state for its built-in
gesture and side-button + wheel horizontal-scroll mode. As a result,
holding a side button more than approximately 2 seconds can prevent the
normal mouse report from emitting a usable button event at all.

HID++ REPROG_CONTROLS_V4 diversion for control IDs 0x0053 and 0x0056
provides real press and release timing for those same controls. Logitech
documents the Signature M650 side buttons as Back/Forward buttons, so
report the diverted controls as BTN_BACK and BTN_FORWARD.

The HID++ 0x1b04 documentation lists those control IDs as Back and
Forward. The driver still verifies that the controls are present in the
device control table and advertised as divertable before changing their
reporting mode.

Link: https://support.logi.com/hc/en-nz/articles/4414473810583-Getting-Started-Signature-M650
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Elliot Douglas <edouglas7358@gmail.com>
---
 drivers/hid/hid-ids.h            |  1 +
 drivers/hid/hid-logitech-hidpp.c | 18 +++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 426ff78c1c03..f4a23662888a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -906,6 +906,7 @@
 #define USB_DEVICE_ID_LOGITECH_Z_10_SPK	0x0a07
 #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
 #define USB_DEVICE_ID_LOGITECH_T651	0xb00c
+#define USB_DEVICE_ID_LOGITECH_SIGNATURE_M650	0xb02a
 #define USB_DEVICE_ID_LOGITECH_DINOVO_EDGE_KBD	0xb309
 #define USB_DEVICE_ID_LOGITECH_CASA_TOUCHPAD	0xbb00
 #define USB_DEVICE_ID_LOGITECH_C007	0xc007
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index f9189e14fb78..8cd762e1eb51 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3624,14 +3624,28 @@ static int hidpp10_extra_mouse_buttons_raw_event(struct hidpp_device *hidpp,
 
 #define HIDPP_REPROG_CONTROLS_EVENT_DIVERTED		0x00
 
+#define HIDPP_REPROG_CONTROL_BACK			0x0053
+#define HIDPP_REPROG_CONTROL_FORWARD			0x0056
+
 struct hidpp_reprog_control_mapping {
 	u16 control;
 	u16 code;
 };
 
+static const struct hidpp_reprog_control_mapping m650_reprog_control_mappings[] = {
+	{ HIDPP_REPROG_CONTROL_BACK, BTN_BACK },
+	{ HIDPP_REPROG_CONTROL_FORWARD, BTN_FORWARD },
+	{ }
+};
+
 static const struct hidpp_reprog_control_mapping *
 hidpp20_reprog_controls_get_mappings(struct hidpp_device *hidpp)
 {
+	switch (hidpp->hid_dev->product) {
+	case USB_DEVICE_ID_LOGITECH_SIGNATURE_M650:
+		return m650_reprog_control_mappings;
+	}
+
 	return NULL;
 }
 
@@ -4911,7 +4925,9 @@ static const struct hid_device_id hidpp_devices[] = {
 	{ /* MX Vertical mouse over Bluetooth */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb020) },
 	{ /* Signature M650 over Bluetooth */
-	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb02a) },
+	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
+			       USB_DEVICE_ID_LOGITECH_SIGNATURE_M650),
+	  .driver_data = HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS },
 	{ /* MX Master 3 mouse over Bluetooth */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb023) },
 	{ /* MX Anywhere 3 mouse over Bluetooth */
-- 
2.55.0

Re: [PATCH v3 1/2] HID: logitech-hidpp: add HID++ 2.0 reprogrammable button support

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-after-free of `hidpp->input` when `input_register_device` fails.
- [High] Out-of-bounds read in `hidpp_raw_hidpp_event()` when copying the report structure.
--

--- Patch [1]: [PATCH v3 1/2] HID: logitech-hidpp: add HID++ 2.0 reprogrammable button support ---
commit 8ed227c4b30632c93149b188c8de90eb6920c093
Author: Elliot Douglas <edouglas7358@gmail.com>

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

> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 70ba1a5e40d8e..f9189e14fb780 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
[ ... ]
> @@ -3601,6 +3606,195 @@ static int hidpp10_extra_mouse_buttons_raw_event(struct hidpp_device *hidpp,
>  	return 1;
>  }
>  
> +/* -------------------------------------------------------------------------- */
> +/* HID++2.0 reprogrammable controls                                           */
> +/* -------------------------------------------------------------------------- */
[ ... ]
> +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 ||
> +	    !hidpp->reprog_controls ||
> +	    hidpp->reprog_controls_feature_index == 0xff)
> +		return 0;

[Severity: High]
This isn't a bug introduced by this patch, but could this lead to a
use-after-free of hidpp->input if input registration fails?

Looking at hidpp_connect_event() in drivers/hid/hid-logitech-hidpp.c, if
input_register_device() fails, it frees the input device but leaves the
global hidpp->input pointer dangling:

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

Since the device is actively receiving interrupts, incoming reports can trigger
raw event handlers like this newly added hidpp20_reprog_controls_raw_event().
The handler bypasses the !hidpp->input check because the pointer is not NULL,
and subsequently calling input_report_key() with the freed pointer could cause
a use-after-free.

> +
> +	if (size < HIDPP_REPORT_LONG_LENGTH ||
> +	    report->fap.feature_index != hidpp->reprog_controls_feature_index ||
> +	    report->fap.funcindex_clientid != HIDPP_REPROG_CONTROLS_EVENT_DIVERTED)
> +		return 0;
[ ... ]
> @@ -3971,6 +4168,10 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
>  			return ret;
>  	}
>  

[Severity: High]
This is a pre-existing issue, but can the struct assignment at the top of
hidpp_raw_hidpp_event() read out of bounds?

When a matching response is found, hidpp_raw_hidpp_event() performs this
assignment:

	if (hidpp_match_answer(question, report) ||
			hidpp_match_error(question, report)) {
		*answer = *report;
		hidpp->answer_available = true;
		wake_up(&hidpp->wait);

The struct hidpp_report is 64 bytes in size. However, the report pointer is cast
directly from the incoming data buffer. If a device only supports SHORT
(7 bytes) or LONG (20 bytes) reports, the data buffer will be sized accordingly.
Does the 64-byte struct copy read out of bounds from the smaller heap buffer
when the incoming report is matched?

> +	ret = hidpp20_reprog_controls_raw_event(hidpp, data, size);
> +	if (ret != 0)
> +		return ret;
> +
>  	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/20260812195842.77724-1-edouglas7358@gmail.com?part=1

Re: [PATCH v3 2/2] HID: logitech-hidpp: enable reprogrammable buttons on Signature M650

[sashiko-bot]

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 `hidpp_probe()` error path.
--

--- Patch [2]: [PATCH v3 2/2] HID: logitech-hidpp: enable reprogrammable buttons on Signature M650 ---
commit 21d2c84d622f6c1b7cd8dfbe446bc728f3b8bf10
Author: Elliot Douglas <edouglas7358@gmail.com>

HID: logitech-hidpp: enable reprogrammable buttons on Signature M650

This patch enables reprogrammable buttons on the Signature M650 mouse over
Bluetooth. It handles the short click-like events emitted for side buttons
by using HID++ REPROG_CONTROLS_V4 diversion to provide real press and
release timing. The diverted controls are reported as BTN_BACK and
BTN_FORWARD.

Link: https://support.logi.com/hc/en-nz/articles/4414473810583-Getting-Started-Signature-M650

> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
[ ... ]
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
[ ... ]

While reviewing this file, I noticed a pre-existing issue in hidpp_probe():

> hid_hw_start_fail:
> 	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 does this error path need to cancel
reset_hi_res_work?

During hidpp_probe(), hid_device_io_start() is called, which enables
incoming packets. If a device with the HIDPP_QUIRK_RESET_HI_RES_SCROLL
quirk sends a battery event before the probe finishes, the event handler
can call:

    schedule_work(&hidpp->reset_hi_res_work);

If a subsequent operation fails and the code branches to hid_hw_start_fail,
it cancels hidpp->work but does not cancel hidpp->reset_hi_res_work.

Since devres automatically frees the hidpp_device struct when probe()
returns, can this pending work item execute hidpp_reset_hi_res_handler()
and cause a use-after-free when it dereferences the freed memory?

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

Re: [PATCH v3 1/2] HID: logitech-hidpp: add HID++ 2.0 reprogrammable button support

[Bastien Nocera]

On Wed, 2026-08-12 at 12:58 -0700, Elliot Douglas wrote:
> 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.
> 
> Add a quirk-gated event path for those controls. The handler
> temporarily
> diverts verified per-product controls, parses divertedButtonsEvent as
> the
> current pressed-control list, and reports the corresponding evdev key
> state
> for every mapped control.
> 
> Keep the control mappings in per-product arrays so adding support for
> another mouse does not change the evdev capabilities advertised by
> already-supported devices.
> 
> Documentation for feature 0x1b04 describes divertedButtonsEvent as a
> list
> of currently pressed diverted buttons, which is the event format
> handled
> here.
> 
> Link:
> https://lekensteyn.nl/files/logitech/x1b04_specialkeysmsebuttons.html
> Signed-off-by: Elliot Douglas <edouglas7358@gmail.com>

My earlier
Reviewed-by: Bastien Nocera <hadess@hadess.net>
was for both patches in the patch set :)

Thanks for v3.

> ---
>  drivers/hid/hid-logitech-hidpp.c | 205
> +++++++++++++++++++++++++++++++
>  1 file changed, 205 insertions(+)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 70ba1a5e40d8..f9189e14fb78 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -76,6 +76,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
>  #define HIDPP_QUIRK_HI_RES_SCROLL_1P0		BIT(28)
>  #define HIDPP_QUIRK_WIRELESS_STATUS		BIT(29)
>  #define HIDPP_QUIRK_RESET_HI_RES_SCROLL		BIT(30)
> +#define HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS	BIT(31)
>  
>  /* These are just aliases for now */
>  #define HIDPP_QUIRK_KBD_SCROLL_WHEEL HIDPP_QUIRK_HIDPP_WHEELS
> @@ -178,6 +179,8 @@ struct hidpp_scroll_counter {
>  	unsigned long long last_time;
>  };
>  
> +struct hidpp_reprog_control_mapping;
> +
>  struct hidpp_device {
>  	struct hid_device *hid_dev;
>  	struct input_dev *input;
> @@ -205,6 +208,8 @@ struct hidpp_device {
>  	struct hidpp_scroll_counter vertical_wheel_counter;
>  
>  	u8 wireless_feature_index;
> +	u8 reprog_controls_feature_index;
> +	const struct hidpp_reprog_control_mapping *reprog_controls;
>  
>  	int hires_wheel_multiplier;
>  	u8 hires_wheel_feature_index;
> @@ -3601,6 +3606,195 @@ static int
> hidpp10_extra_mouse_buttons_raw_event(struct hidpp_device *hidpp,
>  	return 1;
>  }
>  
> +/* -----------------------------------------------------------------
> --------- */
> +/* HID++2.0 reprogrammable
> controls                                           */
> +/* -----------------------------------------------------------------
> --------- */
> +
> +#define HIDPP_PAGE_REPROG_CONTROLS_V4			0x1b04
> +
> +#define HIDPP_REPROG_CONTROLS_GET_COUNT			0x00
> +#define HIDPP_REPROG_CONTROLS_GET_CID_INFO		0x10
> +#define HIDPP_REPROG_CONTROLS_SET_CONTROL_REPORTING	0x30
> +
> +#define HIDPP_REPROG_CONTROLS_FLAG_MOUSE		BIT(0)
> +#define HIDPP_REPROG_CONTROLS_FLAG_DIVERT		BIT(5)
> +
> +#define HIDPP_REPROG_CONTROLS_TEMPORARY_DIVERTED	BIT(0)
> +#define HIDPP_REPROG_CONTROLS_CHANGE_TEMPORARY_DIVERT	BIT(1)
> +
> +#define HIDPP_REPROG_CONTROLS_EVENT_DIVERTED		0x00
> +
> +struct hidpp_reprog_control_mapping {
> +	u16 control;
> +	u16 code;
> +};
> +
> +static const struct hidpp_reprog_control_mapping *
> +hidpp20_reprog_controls_get_mappings(struct hidpp_device *hidpp)
> +{
> +	return NULL;
> +}
> +
> +static int hidpp20_reprog_controls_get_count(struct hidpp_device
> *hidpp)
> +{
> +	struct hidpp_report response;
> +	u8 feature_index = hidpp->reprog_controls_feature_index;
> +	u8 cmd = HIDPP_REPROG_CONTROLS_GET_COUNT;
> +	int ret;
> +
> +	ret = hidpp_send_fap_command_sync(hidpp, feature_index, cmd,
> NULL, 0,
> +					  &response);
> +	if (ret > 0)
> +		return -EPROTO;
> +	if (ret)
> +		return ret;
> +
> +	return response.fap.params[0];
> +}
> +
> +static int hidpp20_reprog_controls_get_cid_info(struct hidpp_device
> *hidpp,
> +						u8 index, u16
> *control,
> +						u8 *flags)
> +{
> +	struct hidpp_report response;
> +	u8 feature_index = hidpp->reprog_controls_feature_index;
> +	u8 cmd = HIDPP_REPROG_CONTROLS_GET_CID_INFO;
> +	int ret;
> +
> +	ret = hidpp_send_fap_command_sync(hidpp, feature_index, cmd,
> &index,
> +					  sizeof(index), &response);
> +	if (ret > 0)
> +		return -EPROTO;
> +	if (ret)
> +		return ret;
> +
> +	*control = get_unaligned_be16(&response.fap.params[0]);
> +	*flags = response.fap.params[4];
> +
> +	return 0;
> +}
> +
> +static bool hidpp20_reprog_controls_find_control(struct hidpp_device
> *hidpp,
> +						 u16 control)
> +{
> +	int count, ret;
> +	u16 cid;
> +	u8 flags;
> +	int i;
> +
> +	count = hidpp20_reprog_controls_get_count(hidpp);
> +	if (count < 0)
> +		return false;
> +
> +	for (i = 0; i < count; i++) {
> +		ret = hidpp20_reprog_controls_get_cid_info(hidpp, i,
> &cid,
> +							   &flags);
> +		if (ret)
> +			return false;
> +
> +		if (cid == control)
> +			return (flags &
> HIDPP_REPROG_CONTROLS_FLAG_MOUSE) &&
> +			       (flags &
> HIDPP_REPROG_CONTROLS_FLAG_DIVERT);
> +	}
> +
> +	return false;
> +}
> +
> +static int hidpp20_reprog_controls_set_control_reporting(struct
> hidpp_device *hidpp,
> +							 u16
> control, u8 flags)
> +{
> +	struct hidpp_report response;
> +	u8 params[5];
> +
> +	put_unaligned_be16(control, &params[0]);
> +	params[2] = flags;
> +	put_unaligned_be16(control, &params[3]);
> +
> +	return hidpp_send_fap_command_sync(hidpp,
> +					   hidpp-
> >reprog_controls_feature_index,
> +					  
> HIDPP_REPROG_CONTROLS_SET_CONTROL_REPORTING,
> +					   params, sizeof(params),
> &response);
> +}
> +
> +static void hidpp20_reprog_controls_connect(struct hidpp_device
> *hidpp)
> +{
> +	const struct hidpp_reprog_control_mapping *mapping;
> +	u8 flags = HIDPP_REPROG_CONTROLS_TEMPORARY_DIVERTED |
> +		   HIDPP_REPROG_CONTROLS_CHANGE_TEMPORARY_DIVERT;
> +
> +	if (!(hidpp->quirks &
> HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS))
> +		return;
> +
> +	if (!hidpp->reprog_controls)
> +		return;
> +
> +	if (hidpp_root_get_feature(hidpp,
> HIDPP_PAGE_REPROG_CONTROLS_V4,
> +				   &hidpp-
> >reprog_controls_feature_index))
> +		return;
> +
> +	for (mapping = hidpp->reprog_controls; mapping->control;
> mapping++) {
> +		if (!hidpp20_reprog_controls_find_control(hidpp,
> mapping->control))
> +			continue;
> +
> +		hidpp20_reprog_controls_set_control_reporting(hidpp,
> +							     
> mapping->control,
> +							     
> flags);
> +	}
> +}
> +
> +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 ||
> +	    !hidpp->reprog_controls ||
> +	    hidpp->reprog_controls_feature_index == 0xff)
> +		return 0;
> +
> +	if (size < HIDPP_REPORT_LONG_LENGTH ||
> +	    report->fap.feature_index != hidpp-
> >reprog_controls_feature_index ||
> +	    report->fap.funcindex_clientid !=
> HIDPP_REPROG_CONTROLS_EVENT_DIVERTED)
> +		return 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(controls); i++)
> +		controls[i] = get_unaligned_be16(&report-
> >fap.params[i * 2]);
> +
> +	for (mapping = hidpp->reprog_controls; mapping->control;
> mapping++) {
> +		pressed = false;
> +
> +		for (j = 0; j < ARRAY_SIZE(controls); j++) {
> +			if (controls[j] == mapping->control) {
> +				pressed = true;
> +				break;
> +			}
> +		}
> +
> +		input_report_key(hidpp->input, mapping->code,
> pressed);
> +	}
> +
> +	input_sync(hidpp->input);
> +
> +	return 1;
> +}
> +
> +static void hidpp20_reprog_controls_populate_input(struct
> hidpp_device *hidpp,
> +						   struct input_dev
> *input_dev)
> +{
> +	const struct hidpp_reprog_control_mapping *mapping;
> +
> +	if (!hidpp->reprog_controls)
> +		return;
> +
> +	for (mapping = hidpp->reprog_controls; mapping->control;
> mapping++)
> +		input_set_capability(input_dev, EV_KEY, mapping-
> >code);
> +}
> +
>  static void hidpp10_extra_mouse_buttons_populate_input(
>  			struct hidpp_device *hidpp, struct input_dev
> *input_dev)
>  {
> @@ -3859,6 +4053,9 @@ static void hidpp_populate_input(struct
> hidpp_device *hidpp,
>  
>  	if (hidpp->quirks & HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS)
>  		hidpp10_extra_mouse_buttons_populate_input(hidpp,
> input);
> +
> +	if (hidpp->quirks & HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS)
> +		hidpp20_reprog_controls_populate_input(hidpp,
> input);
>  }
>  
>  static int hidpp_input_configured(struct hid_device *hdev,
> @@ -3971,6 +4168,10 @@ static int hidpp_raw_hidpp_event(struct
> hidpp_device *hidpp, u8 *data,
>  			return ret;
>  	}
>  
> +	ret = hidpp20_reprog_controls_raw_event(hidpp, data, size);
> +	if (ret != 0)
> +		return ret;
> +
>  	if (hidpp->quirks & HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS)
> {
>  		ret = hidpp10_consumer_keys_raw_event(hidpp, data,
> size);
>  		if (ret != 0)
> @@ -4264,6 +4465,8 @@ static void hidpp_connect_event(struct
> work_struct *work)
>  			return;
>  	}
>  
> +	hidpp20_reprog_controls_connect(hidpp);
> +
>  	if (hidpp->quirks & HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS)
> {
>  		ret = hidpp10_consumer_keys_connect(hidpp);
>  		if (ret)
> @@ -4436,6 +4639,8 @@ static int hidpp_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
>  	hidpp->hid_dev = hdev;
>  	hidpp->name = hdev->name;
>  	hidpp->quirks = id->driver_data;
> +	hidpp->reprog_controls_feature_index = 0xff;
> +	hidpp->reprog_controls =
> hidpp20_reprog_controls_get_mappings(hidpp);
>  	hid_set_drvdata(hdev, hidpp);
>  
>  	ret = hid_parse(hdev);