Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 0/3] HID: Apple Magic Keyboard/Trackpad battery over Bluetooth
@ 2026-07-14 10:12 Alec Hall
  2026-07-14 10:12 ` [PATCH 1/3] HID: apple: report Magic Keyboard " Alec Hall
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Alec Hall @ 2026-07-14 10:12 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: linux-kernel, Alec Hall

Apple's Magic Keyboard and Magic Trackpad 2 / Magic Mouse 2 report their
battery to the host, but the kernel only surfaces it over USB; over
Bluetooth the power_supply is stuck at 0%, even though these devices are
used over Bluetooth nearly all the time.

Both drivers already fetch the battery (a GET_REPORT for input report
0x90) but only over USB. Enabling it over Bluetooth naively hard-locks the
machine: the fetch runs from a timer callback (atomic context) and the
uhid transport sleeps inside hid_hw_request(). Each driver's patch
therefore moves the fetch to a workqueue before enabling it over
Bluetooth. The last patch teaches hid-magicmouse to report charge status
from the device's status byte.

Patch 3 interprets a vendor status byte whose bit meanings were determined
empirically; I can rework it as a report-descriptor fixup exposing
HID_BAT_CHARGING instead if reviewers prefer.

Tested on a Magic Keyboard 2021 and a Magic Trackpad 2 over BlueZ (uhid).

Alec Hall (3):
  HID: apple: report Magic Keyboard battery over Bluetooth
  HID: magicmouse: report battery over Bluetooth
  HID: magicmouse: report charge status over Bluetooth

 drivers/hid/hid-apple.c      | 36 ++++++-------
 drivers/hid/hid-magicmouse.c | 99 ++++++++++++++++++++++++++----------
 2 files changed, 92 insertions(+), 43 deletions(-)


base-commit: b7556c8e713c88596046a906c7c4385218d44736
-- 
2.55.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] HID: apple: report Magic Keyboard battery over Bluetooth
  2026-07-14 10:12 [PATCH 0/3] HID: Apple Magic Keyboard/Trackpad battery over Bluetooth Alec Hall
@ 2026-07-14 10:12 ` Alec Hall
  2026-07-14 10:12 ` [PATCH 2/3] HID: magicmouse: report " Alec Hall
  2026-07-14 10:12 ` [PATCH 3/3] HID: magicmouse: report charge status " Alec Hall
  2 siblings, 0 replies; 6+ messages in thread
From: Alec Hall @ 2026-07-14 10:12 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: linux-kernel, Alec Hall

The Magic Keyboard answers a GET_REPORT for battery input report 0x90
over Bluetooth exactly as it does over USB, but battery reporting was
only ever enabled for USB: the Bluetooth device-table entry lacks the
APPLE_RDESC_BATTERY quirk, so apple_fetch_battery() returns early and
the kernel's power_supply is stuck at 0%.

The USB-only restriction is not cosmetic. apple_fetch_battery() issues
its GET_REPORT through hid_hw_request(), and over Bluetooth that goes
through the uhid transport, whose raw_request sleeps waiting on user
space. The fetch was driven from a timer callback
(apple_battery_timer_tick), which runs in atomic softirq context where
sleeping is forbidden, so requesting the battery there deadlocks the
machine:

    run_timer_softirq
     __run_timer_base
      apple_battery_timer_tick
       __hid_request
        uhid_hid_raw_request   /* sleeps in atomic context */

Move the periodic battery fetch to a delayed work item, which runs in
process context and may sleep, and set APPLE_RDESC_BATTERY on the
Bluetooth Magic Keyboard 2021 entry. The descriptor fixup guarded by
the quirk only fires for the 83-byte USB report descriptor, so enabling
the quirk over Bluetooth does not disturb the (different) Bluetooth
descriptor. Also stop returning early once capacity reaches max: over
Bluetooth the device sends no unsolicited updates, so the driver must
keep polling to notice the level fall after a full charge.

Only the plain Magic Keyboard 2021 is enabled and tested here; the
fingerprint, numpad and 2024 Bluetooth variants likely need the same
change but were not available to test.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Alec Hall <signshop.alec@gmail.com>
---
 drivers/hid/hid-apple.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index bf7dd0fbf249..e46a59844a30 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/timer.h>
+#include <linux/workqueue.h>
 #include <linux/string.h>
 #include <linux/leds.h>
 #include <dt-bindings/leds/common.h>
@@ -116,7 +117,7 @@ struct apple_sc {
 	unsigned int fn_on;
 	unsigned int fn_found;
 	DECLARE_BITMAP(pressed_numlock, KEY_CNT);
-	struct timer_list battery_timer;
+	struct delayed_work battery_work;
 	struct apple_sc_backlight *backlight;
 };
 
@@ -635,9 +636,6 @@ static int apple_fetch_battery(struct hid_device *hdev)
 	if (!report || report->maxfield < 1)
 		return -1;
 
-	if (bat->capacity == bat->max)
-		return -1;
-
 	hid_hw_request(hdev, report, HID_REQ_GET_REPORT);
 	return 0;
 #else
@@ -645,15 +643,20 @@ static int apple_fetch_battery(struct hid_device *hdev)
 #endif
 }
 
-static void apple_battery_timer_tick(struct timer_list *t)
+static void apple_battery_work(struct work_struct *work)
 {
-	struct apple_sc *asc = timer_container_of(asc, t, battery_timer);
+	struct apple_sc *asc = container_of(work, struct apple_sc, battery_work.work);
 	struct hid_device *hdev = asc->hdev;
 
-	if (apple_fetch_battery(hdev) == 0) {
-		mod_timer(&asc->battery_timer,
-			  jiffies + secs_to_jiffies(APPLE_BATTERY_TIMEOUT_SEC));
-	}
+	/*
+	 * Runs in process context (workqueue), so the battery GET_REPORT is
+	 * allowed to sleep. This is required for the uhid/Bluetooth transport,
+	 * whose raw_request blocks waiting on userspace -- unlike a timer_list
+	 * callback, which runs in atomic softirq context and would deadlock.
+	 */
+	if (apple_fetch_battery(hdev) == 0)
+		schedule_delayed_work(&asc->battery_work,
+				      secs_to_jiffies(APPLE_BATTERY_TIMEOUT_SEC));
 }
 
 /*
@@ -968,10 +971,9 @@ static int apple_probe(struct hid_device *hdev,
 	}
 
 	if (quirks & APPLE_RDESC_BATTERY) {
-		timer_setup(&asc->battery_timer, apple_battery_timer_tick, 0);
-		mod_timer(&asc->battery_timer,
-			  jiffies + secs_to_jiffies(APPLE_BATTERY_TIMEOUT_SEC));
-		apple_fetch_battery(hdev);
+		INIT_DELAYED_WORK(&asc->battery_work, apple_battery_work);
+		/* Kick an initial fetch; the work re-arms itself every timeout. */
+		schedule_delayed_work(&asc->battery_work, 0);
 	}
 
 	if (quirks & APPLE_BACKLIGHT_CTL)
@@ -987,7 +989,7 @@ static int apple_probe(struct hid_device *hdev,
 
 out_err:
 	if (quirks & APPLE_RDESC_BATTERY)
-		timer_delete_sync(&asc->battery_timer);
+		cancel_delayed_work_sync(&asc->battery_work);
 
 	hid_hw_stop(hdev);
 	return ret;
@@ -998,7 +1000,7 @@ static void apple_remove(struct hid_device *hdev)
 	struct apple_sc *asc = hid_get_drvdata(hdev);
 
 	if (asc->quirks & APPLE_RDESC_BATTERY)
-		timer_delete_sync(&asc->battery_timer);
+		cancel_delayed_work_sync(&asc->battery_work);
 
 	hid_hw_stop(hdev);
 }
@@ -1201,7 +1203,7 @@ static const struct hid_device_id apple_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_2021),
 		.driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK | APPLE_RDESC_BATTERY },
 	{ HID_BLUETOOTH_DEVICE(BT_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_2021),
-		.driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK },
+		.driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK | APPLE_RDESC_BATTERY },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_2021),
 		.driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK | APPLE_RDESC_BATTERY },
 	{ HID_BLUETOOTH_DEVICE(BT_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_2021),
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] HID: magicmouse: report battery over Bluetooth
  2026-07-14 10:12 [PATCH 0/3] HID: Apple Magic Keyboard/Trackpad battery over Bluetooth Alec Hall
  2026-07-14 10:12 ` [PATCH 1/3] HID: apple: report Magic Keyboard " Alec Hall
@ 2026-07-14 10:12 ` Alec Hall
  2026-07-14 10:12 ` [PATCH 3/3] HID: magicmouse: report charge status " Alec Hall
  2 siblings, 0 replies; 6+ messages in thread
From: Alec Hall @ 2026-07-14 10:12 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: linux-kernel, Alec Hall

magicmouse_fetch_battery() only runs for USB Magic Mouse 2 / Magic
Trackpad 2. Over Bluetooth the kernel never queries the battery and the
power_supply reports 0%, even though the device answers a GET_REPORT for
battery input report 0x90. (A Magic Trackpad 2 appears to work while
charging only because it then pushes battery reports unsolicited; off
the charger the reading goes stale.)

As with hid-apple, the fetch cannot run from the battery timer over
Bluetooth: hid_hw_request() sleeps on the uhid transport, but
magicmouse_battery_timer_tick() runs in atomic softirq context. Move
the periodic fetch to a delayed work item, add magicmouse_has_battery()
and use it to arm the fetch for Magic Mouse 2 / Magic Trackpad 2
regardless of transport, and keep polling once full so the reading
stays current over Bluetooth.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Alec Hall <signshop.alec@gmail.com>
---
 drivers/hid/hid-magicmouse.c | 62 +++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 802a3479e24b..6d0e76314b10 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -123,7 +123,7 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
  * @tracking_ids: Mapping of current touch input data to @touches.
  * @hdev: Pointer to the underlying HID device.
  * @work: Workqueue to handle initialization retry for quirky devices.
- * @battery_timer: Timer for obtaining battery level information.
+ * @battery_work: Delayed work for obtaining battery level information.
  */
 struct magicmouse_sc {
 	struct input_dev *input;
@@ -148,7 +148,7 @@ struct magicmouse_sc {
 
 	struct hid_device *hdev;
 	struct delayed_work work;
-	struct timer_list battery_timer;
+	struct delayed_work battery_work;
 };
 
 static int magicmouse_firm_touch(struct magicmouse_sc *msc)
@@ -828,6 +828,19 @@ static bool is_usb_magictrackpad2(__u32 vendor, __u32 product)
 	       product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC;
 }
 
+/*
+ * Magic Mouse 2 and Magic Trackpad 2 expose a battery over both USB and
+ * Bluetooth (the pre-2 Magic Mouse/Trackpad do not). Transport-agnostic:
+ * over Bluetooth the vendor is BT_VENDOR_ID_APPLE but the product id matches.
+ */
+static bool magicmouse_has_battery(struct hid_device *hdev)
+{
+	return hdev->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 ||
+	       hdev->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2_USBC ||
+	       hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 ||
+	       hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC;
+}
+
 static int magicmouse_fetch_battery(struct hid_device *hdev)
 {
 #ifdef CONFIG_HID_BATTERY_STRENGTH
@@ -836,9 +849,7 @@ static int magicmouse_fetch_battery(struct hid_device *hdev)
 	struct hid_battery *bat;
 
 	bat = hid_get_battery(hdev);
-	if (!bat ||
-	    (!is_usb_magicmouse2(hdev->vendor, hdev->product) &&
-	     !is_usb_magictrackpad2(hdev->vendor, hdev->product)))
+	if (!bat || !magicmouse_has_battery(hdev))
 		return -1;
 
 	report_enum = &hdev->report_enum[bat->report_type];
@@ -847,9 +858,6 @@ static int magicmouse_fetch_battery(struct hid_device *hdev)
 	if (!report || report->maxfield < 1)
 		return -1;
 
-	if (bat->capacity == bat->max)
-		return -1;
-
 	hid_hw_request(hdev, report, HID_REQ_GET_REPORT);
 	return 0;
 #else
@@ -857,15 +865,21 @@ static int magicmouse_fetch_battery(struct hid_device *hdev)
 #endif
 }
 
-static void magicmouse_battery_timer_tick(struct timer_list *t)
+static void magicmouse_battery_work(struct work_struct *work)
 {
-	struct magicmouse_sc *msc = timer_container_of(msc, t, battery_timer);
+	struct magicmouse_sc *msc = container_of(work, struct magicmouse_sc,
+						 battery_work.work);
 	struct hid_device *hdev = msc->hdev;
 
-	if (magicmouse_fetch_battery(hdev) == 0) {
-		mod_timer(&msc->battery_timer,
-			  jiffies + secs_to_jiffies(USB_BATTERY_TIMEOUT_SEC));
-	}
+	/*
+	 * Runs in process context (workqueue), so the battery GET_REPORT may
+	 * sleep. This is required for the uhid/Bluetooth transport, whose
+	 * raw_request blocks on userspace -- unlike a timer_list callback,
+	 * which runs in atomic softirq context and would deadlock.
+	 */
+	if (magicmouse_fetch_battery(hdev) == 0)
+		schedule_delayed_work(&msc->battery_work,
+				      secs_to_jiffies(USB_BATTERY_TIMEOUT_SEC));
 }
 
 static int magicmouse_probe(struct hid_device *hdev,
@@ -900,12 +914,10 @@ static int magicmouse_probe(struct hid_device *hdev,
 		return ret;
 	}
 
-	if (is_usb_magicmouse2(id->vendor, id->product) ||
-	    is_usb_magictrackpad2(id->vendor, id->product)) {
-		timer_setup(&msc->battery_timer, magicmouse_battery_timer_tick, 0);
-		mod_timer(&msc->battery_timer,
-			  jiffies + secs_to_jiffies(USB_BATTERY_TIMEOUT_SEC));
-		magicmouse_fetch_battery(hdev);
+	if (magicmouse_has_battery(hdev)) {
+		INIT_DELAYED_WORK(&msc->battery_work, magicmouse_battery_work);
+		/* Kick an initial fetch; the work re-arms itself each timeout. */
+		schedule_delayed_work(&msc->battery_work, 0);
 	}
 
 	if (is_usb_magicmouse2(id->vendor, id->product) ||
@@ -973,9 +985,8 @@ static int magicmouse_probe(struct hid_device *hdev,
 
 	return 0;
 err_stop_hw:
-	if (is_usb_magicmouse2(id->vendor, id->product) ||
-	    is_usb_magictrackpad2(id->vendor, id->product))
-		timer_delete_sync(&msc->battery_timer);
+	if (magicmouse_has_battery(hdev))
+		cancel_delayed_work_sync(&msc->battery_work);
 
 	hid_hw_stop(hdev);
 	return ret;
@@ -987,9 +998,8 @@ static void magicmouse_remove(struct hid_device *hdev)
 
 	if (msc) {
 		cancel_delayed_work_sync(&msc->work);
-		if (is_usb_magicmouse2(hdev->vendor, hdev->product) ||
-		    is_usb_magictrackpad2(hdev->vendor, hdev->product))
-			timer_delete_sync(&msc->battery_timer);
+		if (magicmouse_has_battery(hdev))
+			cancel_delayed_work_sync(&msc->battery_work);
 	}
 
 	hid_hw_stop(hdev);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth
  2026-07-14 10:12 [PATCH 0/3] HID: Apple Magic Keyboard/Trackpad battery over Bluetooth Alec Hall
  2026-07-14 10:12 ` [PATCH 1/3] HID: apple: report Magic Keyboard " Alec Hall
  2026-07-14 10:12 ` [PATCH 2/3] HID: magicmouse: report " Alec Hall
@ 2026-07-14 10:12 ` Alec Hall
  2026-07-14 10:25   ` sashiko-bot
  2 siblings, 1 reply; 6+ messages in thread
From: Alec Hall @ 2026-07-14 10:12 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: linux-kernel, Alec Hall

Battery input report 0x90 carries a status byte that the generic HID
battery code does not map to HID_BAT_CHARGING, so charge_status keeps
its POWER_SUPPLY_STATUS_DISCHARGING default and user space never learns
whether the device is on external power (e.g. a Magic Trackpad 2 shows
"discharging" by default even while charging, and cannot report
"discharging" the moment the cable is pulled at 100%).

Parse the status byte in raw_event and set the battery charge status
accordingly (bit 1 = external power, bit 0 = charge complete), notifying
user space with power_supply_changed(). The bit meanings were determined
by observation on a Magic Trackpad 2 rather than from documentation.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Alec Hall <signshop.alec@gmail.com>
---
 drivers/hid/hid-magicmouse.c | 37 ++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 6d0e76314b10..8bb791efef3a 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
+#include <linux/power_supply.h>
 
 #include "hid-ids.h"
 
@@ -60,6 +61,10 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
 #define MOUSE_REPORT_ID    0x29
 #define MOUSE2_REPORT_ID   0x12
 #define DOUBLE_REPORT_ID   0xf7
+/* Battery Input Report 0x90 = [report_id, status, capacity]. */
+#define MAGICMOUSE_BATTERY_REPORT_ID	0x90
+#define MAGICMOUSE_BATTERY_POWERED	0x02	/* external power connected */
+#define MAGICMOUSE_BATTERY_CHARGED	0x01	/* charge complete (on charger) */
 #define USB_BATTERY_TIMEOUT_SEC 60
 
 /* These definitions are not precise, but they're close enough.  (Bits
@@ -383,6 +388,35 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 	}
 }
 
+/*
+ * The battery report carries a charge-status byte the generic HID battery code
+ * doesn't map, so translate the vendor status bits into a power_supply status.
+ * This makes charging/discharging honest -- e.g. "discharging" as soon as the
+ * cable is pulled, even at 100%.
+ */
+static void magicmouse_report_charge_status(struct hid_device *hdev, u8 status)
+{
+#ifdef CONFIG_HID_BATTERY_STRENGTH
+	struct hid_battery *bat = hid_get_battery(hdev);
+	int cs;
+
+	if (!bat || !bat->ps)
+		return;
+
+	if (!(status & MAGICMOUSE_BATTERY_POWERED))
+		cs = POWER_SUPPLY_STATUS_DISCHARGING;
+	else if (status & MAGICMOUSE_BATTERY_CHARGED)
+		cs = POWER_SUPPLY_STATUS_FULL;
+	else
+		cs = POWER_SUPPLY_STATUS_CHARGING;
+
+	if (bat->charge_status != cs) {
+		bat->charge_status = cs;
+		power_supply_changed(bat->ps);
+	}
+#endif
+}
+
 static int magicmouse_raw_event(struct hid_device *hdev,
 		struct hid_report *report, u8 *data, int size)
 {
@@ -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:
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth
  2026-07-14 10:12 ` [PATCH 3/3] HID: magicmouse: report charge status " Alec Hall
@ 2026-07-14 10:25   ` sashiko-bot
  2026-07-14 17:17     ` Alec Hall
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-07-14 10:25 UTC (permalink / raw)
  To: Alec Hall; +Cc: dmitry.torokhov, linux-input

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 <signshop.alec@gmail.com>

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().

> 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

[ ... ]

> @@ -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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth
  2026-07-14 10:25   ` sashiko-bot
@ 2026-07-14 17:17     ` Alec Hall
  0 siblings, 0 replies; 6+ messages in thread
From: Alec Hall @ 2026-07-14 17:17 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Alec Hall

> Sashiko AI review found 2 potential issue(s) to consider:
> - [High] NULL dereference of `msc->input` in magicmouse_raw_event /
>   magicmouse_emit_touch
> - [High] Unbounded recursion via nested DOUBLE_REPORT_ID (0xf7) reports

Thanks for the review. Both are pre-existing and are neither introduced nor
worsened by this series, but they're real and I'm happy to fix them.

> can this function trigger a kernel panic due to a NULL pointer dereference
> on msc->input?

This series does not add any new use of msc->input. The battery report path
added in patch 3 returns through the switch "default" case (report 0x90 is
not a touch report), i.e. before the input->id.product access at the end of
magicmouse_raw_event(), and magicmouse_report_charge_status() only calls
hid_get_battery() -- it never touches msc->input.

That said, the pre-existing exposure you describe (a device that takes the
early return in magicmouse_probe() and then sends a touch-shaped report) is
real, and an early "if (!msc->input) return 0;" at the top of
magicmouse_raw_event() is a sensible guard.

> does this function have a potential stack overflow via unbounded recursion?

Also untouched by this series, but yes, the self-recursion on DOUBLE_REPORT_ID
is unbounded on malicious input and worth bounding (reworking it into a loop,
or capping the nesting depth).

Since both are independent of the battery feature, I'd prefer to send them as
a separate hardening series rather than fold them into this one. Dmitry, would
you like me to post that now, or do you consider these out of scope here? I'm
also happy to add just the msc->input guard to this series if you'd rather see
it alongside patch 3, since that patch touches magicmouse_raw_event().

Thanks,
Alec Hall

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-14 17:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 10:12 [PATCH 0/3] HID: Apple Magic Keyboard/Trackpad battery over Bluetooth Alec Hall
2026-07-14 10:12 ` [PATCH 1/3] HID: apple: report Magic Keyboard " Alec Hall
2026-07-14 10:12 ` [PATCH 2/3] HID: magicmouse: report " Alec Hall
2026-07-14 10:12 ` [PATCH 3/3] HID: magicmouse: report charge status " Alec Hall
2026-07-14 10:25   ` sashiko-bot
2026-07-14 17:17     ` Alec Hall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox