Re: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth
From: sashiko-bot@kernel.org
Date: 2026-07-14 10:25:40
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] The driver unconditionally dereferences `msc->input` in `magicmouse_raw_event` and `magicmouse_emit_touch`, leading to a kernel panic if `msc->input` is NULL. - [High] A malicious device can send a deeply nested sequence of `DOUBLE_REPORT_ID` (0xf7) reports in a single packet, causing `magicmouse_raw_event` to recurse until it overflows the kernel stack. --
--- Patch [3]: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth ---commit 7588bfd842ef79dc73b3904f9e721e709d0dc4fe Author: Alec Hall [off-list ref] HID: magicmouse: report charge status over Bluetooth This commit adds parsing for the status byte in raw battery input reports for the Magic Mouse/Trackpad. It extracts external power and charge complete bits and reports them to user space via power_supply_changed().
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index 6d0e76314b105..8bb791efef3a8 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -394,6 +428,9 @@ static int magicmouse_raw_event(struct hid_device *hdev, if (size < 1) return 0; + if (data[0] == MAGICMOUSE_BATTERY_REPORT_ID && size >= 3) + magicmouse_report_charge_status(hdev, data[1]); + switch (data[0]) { case TRACKPAD_REPORT_ID: case TRACKPAD2_BT_REPORT_ID:
[Severity: High]
This is a pre-existing issue, but can this function trigger a kernel panic
due to a NULL pointer dereference on msc->input?
If a malicious USB device spoofs the Magic Mouse or Trackpad 2 product IDs,
magicmouse_probe() can return early without validating !msc->input:
drivers/hid/hid-magicmouse.c:magicmouse_probe() {
if (is_usb_magicmouse2(id->vendor, id->product) ||
(is_usb_magictrackpad2(id->vendor, id->product) &&
hdev->type != HID_TYPE_USBMOUSE))
return 0;
if (!msc->input) {
...
}
}
When this device later sends a raw HID report, magicmouse_raw_event() will
unconditionally dereference input->id.product at the end of the function:
if (input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE || ...
This might also happen if hid_hw_start() enables interrupts and processes
incoming raw events before hid_connect() finishes initializing msc->input.
Could a check for msc->input be added to prevent this?
[Severity: High]
This is also a pre-existing issue, but does this function have a potential
stack overflow vulnerability via unbounded recursion?
If a malicious device sends a nested sequence of DOUBLE_REPORT_ID (0xf7)
reports in a single packet with a size payload of 0, the recursive calls
will consume only 2 bytes of the payload per call:
drivers/hid/hid-magicmouse.c:magicmouse_raw_event() {
...
case DOUBLE_REPORT_ID:
...
magicmouse_raw_event(hdev, report, data + 2, data[1]);
magicmouse_raw_event(hdev, report, data + 2 + data[1],
size - 2 - data[1]);
return 0;
...
}
With a maximum USB/HID packet size of 16KB, could this result in up to
8,192 recursive calls and exhaust the kernel stack?
Is there a way to limit the maximum recursion depth here?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714101235.99447-1-signshop.alec@gmail.com?part=3