* [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