* [PATCH] HID: magicmouse: prevent unbounded recursion in magicmouse_raw_event()
@ 2026-07-15 5:35 Jose Villaseñor Montfort
2026-07-15 23:04 ` Alec Hall
0 siblings, 1 reply; 2+ messages in thread
From: Jose Villaseñor Montfort @ 2026-07-15 5:35 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Jose Villaseñor Montfort
magicmouse_raw_event() handles DOUBLE_REPORT_ID (0xf7) packets, which pack
two touch reports into one, by splitting the packet and calling itself on
each half. The only guard against runaway recursion is a "size < 1" check,
which stops zero-sized calls but does not bound the recursion depth.
A malicious HID device that matches this driver can send a report starting
with DOUBLE_REPORT_ID and filled with the sequence [0xf7, 0x00]. Each level
consumes two bytes and recurses on the remainder, so an incoming report of
up to HID_MAX_BUFFER_SIZE (16 KiB) drives roughly 8000 nested calls. That
easily exhausts the 16 KiB kernel stack, leading to a stack overflow: a
panic with CONFIG_VMAP_STACK, or memory corruption without it.
A double report only ever wraps two normal reports; it is never
legitimately nested. Refuse to re-enter the DOUBLE_REPORT_ID case from a
recursive call so the recursion depth is bounded to two, while all valid
packets keep being parsed exactly as before.
Fixes: a462230e16ac ("HID: magicmouse: enable Magic Trackpad support")
Link: https://lore.kernel.org/linux-input/20260706181347.700DB1F00A3F@smtp.kernel.org/
Cc: stable@vger.kernel.org
Signed-off-by: Jose Villaseñor Montfort <pepemontfort@gmail.com>
---
Noticed while reviewing hid-magicmouse.c during the discussion of the
parallel Magic Trackpad USB-C battery work [1]. The recursion issue is
independent of that series and is sent as its own fix.
I also considered refactoring the report parsing into a non-recursive
helper that dispatches the two sub-reports iteratively, which removes the
recursion entirely. That is a larger and more intrusive change; this
minimal guard keeps the diff small and is trivial to backport, so I went
with it. Happy to switch to the refactor if maintainers prefer it.
[1] https://lore.kernel.org/linux-input/20260706175507.47288-1-andfed.net@gmail.com/
drivers/hid/hid-magicmouse.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 802a3479e..97562765a 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -383,8 +383,8 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
}
}
-static int magicmouse_raw_event(struct hid_device *hdev,
- struct hid_report *report, u8 *data, int size)
+static int __magicmouse_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data, int size, bool nested)
{
struct magicmouse_sc *msc = hid_get_drvdata(hdev);
struct input_dev *input = msc->input;
@@ -495,6 +495,15 @@ static int magicmouse_raw_event(struct hid_device *hdev,
* packet.
*/
+ /*
+ * A double report only ever wraps two normal reports, so it is
+ * never nested. Refuse to recurse a second time; otherwise a
+ * malicious device could chain DOUBLE_REPORT_ID packets to drive
+ * unbounded recursion and overflow the kernel stack.
+ */
+ if (nested)
+ return 0;
+
/* Ensure that we have at least 2 elements (report type and size) */
if (size < 2)
return 0;
@@ -506,9 +515,9 @@ static int magicmouse_raw_event(struct hid_device *hdev,
return 0;
}
- magicmouse_raw_event(hdev, report, data + 2, data[1]);
- magicmouse_raw_event(hdev, report, data + 2 + data[1],
- size - 2 - data[1]);
+ __magicmouse_raw_event(hdev, report, data + 2, data[1], true);
+ __magicmouse_raw_event(hdev, report, data + 2 + data[1],
+ size - 2 - data[1], true);
return 0;
default:
return 0;
@@ -534,6 +543,12 @@ static int magicmouse_raw_event(struct hid_device *hdev,
return 1;
}
+static int magicmouse_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data, int size)
+{
+ return __magicmouse_raw_event(hdev, report, data, size, false);
+}
+
static int magicmouse_event(struct hid_device *hdev, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{
--
2.55.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] HID: magicmouse: prevent unbounded recursion in magicmouse_raw_event()
2026-07-15 5:35 [PATCH] HID: magicmouse: prevent unbounded recursion in magicmouse_raw_event() Jose Villaseñor Montfort
@ 2026-07-15 23:04 ` Alec Hall
0 siblings, 0 replies; 2+ messages in thread
From: Alec Hall @ 2026-07-15 23:04 UTC (permalink / raw)
To: Jose Villaseñor Montfort, linux-input
Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, Alec Hall
On Tue, Jul 14, 2026, Jose Villaseñor Montfort wrote:
> A double report only ever wraps two normal reports; it is never
> legitimately nested. Refuse to re-enter the DOUBLE_REPORT_ID case from a
> recursive call so the recursion depth is bounded to two [...]
Reviewed the logic: gating re-entry into the DOUBLE_REPORT_ID case on the
new "nested" flag caps recursion at two levels, and since a legitimate
double only ever wraps two non-double reports, valid traffic parses
exactly as before. The guard placement (before the size checks and the
split) and the wrapper preserving the callback signature both look right.
I also stacked this on top of the pending Magic Keyboard/Trackpad battery
series and ran it on a Magic Trackpad 2 over Bluetooth: normal pointer plus
multi-finger gestures (which drive the DOUBLE_REPORT_ID path), and battery
reporting, all keep working with no regressions or splats in dmesg.
Reviewed-by: Alec Hall <signshop.alec@gmail.com>
Tested-by: Alec Hall <signshop.alec@gmail.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-15 23:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 5:35 [PATCH] HID: magicmouse: prevent unbounded recursion in magicmouse_raw_event() Jose Villaseñor Montfort
2026-07-15 23:04 ` Alec Hall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox