Thread (10 messages) 10 messages, 3 authors, 1d ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help