* [PATCH] HID: magicmouse: reject devices that bind without an input device
@ 2026-07-15 19:58 Jose Villaseñor Montfort
2026-07-28 7:25 ` Alec Hall
2026-07-28 18:40 ` [PATCH v2] HID: magicmouse: avoid NULL pointer deref when there is no " Jose Villaseñor Montfort
0 siblings, 2 replies; 5+ messages in thread
From: Jose Villaseñor Montfort @ 2026-07-15 19:58 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Jose Villaseñor Montfort
magicmouse_raw_event() and magicmouse_event() dereference msc->input
(e.g. input->id.product, and via magicmouse_emit_touch() and
magicmouse_emit_buttons()) without checking it for NULL. hid-input only
sets msc->input when the device exposes a usable input device.
magicmouse_probe() guards against this with an "input not registered"
check that fails the probe when msc->input is NULL -- but the USB Magic
Mouse 2 / Magic Trackpad 2 path returns 0 before reaching that check.
A device that binds this driver on that path (for example a malicious
one spoofing an Apple VID/PID) with a report descriptor that does not
produce an input device therefore ends up bound with msc->input == NULL.
A subsequent input report then dereferences the NULL pointer in the
->raw_event / ->event callbacks and panics the kernel.
Move the msc->input check ahead of the early return so it covers every
bind path. Legitimate devices register an input during hid_hw_start()
and are unaffected.
Fixes: 0b91b4e4dae6 ("HID: magicmouse: Report battery level over USB")
Link: https://lore.kernel.org/linux-input/20260714102540.3EB2E1F000E9@smtp.kernel.org/
Cc: stable@vger.kernel.org
Signed-off-by: Jose Villaseñor Montfort <pepemontfort@gmail.com>
---
Surfaced by an automated review of Alec Hall's parallel battery series
(the Link: above), independent of that work. This is a sibling hardening
fix to "HID: magicmouse: prevent unbounded recursion in
magicmouse_raw_event()" [1], which touches the same driver.
I went with fixing the probe path (rejecting a bind without an input)
rather than adding per-callback "if (!msc->input) return 0;" guards,
since a single check at probe covers both ->raw_event and ->event and
addresses the root asymmetry. Happy to switch to per-callback guards if
reviewers prefer that.
[1] https://lore.kernel.org/linux-input/20260715053526.574725-1-pepemontfort@gmail.com/
drivers/hid/hid-magicmouse.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 97562765a..bd6a12e40 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -923,17 +923,25 @@ static int magicmouse_probe(struct hid_device *hdev,
magicmouse_fetch_battery(hdev);
}
- if (is_usb_magicmouse2(id->vendor, id->product) ||
- (is_usb_magictrackpad2(id->vendor, id->product) &&
- hdev->type != HID_TYPE_USBMOUSE))
- return 0;
-
+ /*
+ * The ->raw_event and ->event callbacks dereference msc->input, which
+ * hid-input only populates when the device exposes a usable input.
+ * Reject a device that bound without one -- including on the USB Magic
+ * Mouse 2 / Trackpad 2 path that returns early below -- so a device
+ * (e.g. one spoofing an Apple VID/PID) cannot drive those callbacks
+ * into a NULL pointer dereference.
+ */
if (!msc->input) {
hid_err(hdev, "magicmouse input not registered\n");
ret = -ENOMEM;
goto err_stop_hw;
}
+ if (is_usb_magicmouse2(id->vendor, id->product) ||
+ (is_usb_magictrackpad2(id->vendor, id->product) &&
+ hdev->type != HID_TYPE_USBMOUSE))
+ return 0;
+
switch (id->product) {
case USB_DEVICE_ID_APPLE_MAGICMOUSE:
report = hid_register_report(hdev, HID_INPUT_REPORT, MOUSE_REPORT_ID, 0);
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] HID: magicmouse: reject devices that bind without an input device
2026-07-15 19:58 [PATCH] HID: magicmouse: reject devices that bind without an input device Jose Villaseñor Montfort
@ 2026-07-28 7:25 ` Alec Hall
2026-07-28 18:37 ` Jose Villaseñor Montfort
2026-07-28 18:40 ` [PATCH v2] HID: magicmouse: avoid NULL pointer deref when there is no " Jose Villaseñor Montfort
1 sibling, 1 reply; 5+ messages in thread
From: Alec Hall @ 2026-07-28 7:25 UTC (permalink / raw)
To: pepemontfort; +Cc: jikos, bentiss, linux-input, linux-kernel
On Wed, Jul 15, 2026, Jose Villaseñor Montfort wrote:
> Move the msc->input check ahead of the early return so it covers every
> bind path. Legitimate devices register an input during hid_hw_start()
> and are unaffected.
The second half of that turns out not to hold on real hardware. A USB
Magic Trackpad 2 exposes four HID interfaces, and only the first two
register an input; interfaces 2 and 3 are vendor-defined, hiddev/hidraw
only. With this patch applied (stacked on the pending battery series,
kernel 7.1.5), plugging in a Magic Trackpad 2 over USB rejects both of
them:
magicmouse 0003:05AC:0265.001F: magicmouse input not registered
magicmouse 0003:05AC:0265.0020: magicmouse input not registered
I confirmed with a temporary printk in probe that interfaces 2/3 reach
the moved check with msc->input == NULL and hdev->claimed == 0x6
(hiddev|hidraw, no input), while interfaces 0/1 probe with an input and
claimed == 0x7 and keep working, battery included.
The rejected interfaces then end up bound to nothing -- hid-generic
declines them because a specific driver matches the ID -- so their
hidraw nodes disappear, and every cable plug logs two -ENOMEM probe
failures for a healthy device. Nothing user-facing breaks (touch and
battery live on interfaces 0/1), but the driver has claimed those
interfaces ever since 0b91b4e4dae6, and silently unbinding them with an
error doesn't seem right for a stable-tagged fix.
Bluetooth is unaffected: that path already required an input before the
early return, and a Magic Trackpad 2 + Magic Keyboard over BT show no
change with this patch.
> I went with fixing the probe path (rejecting a bind without an input)
> rather than adding per-callback "if (!msc->input) return 0;" guards,
> since a single check at probe covers both ->raw_event and ->event and
> addresses the root asymmetry. Happy to switch to per-callback guards if
> reviewers prefer that.
Given the above I think the per-callback guard is the better shape after
all: it keeps the legitimate input-less USB interfaces bound exactly as
today, while still closing the NULL deref in ->raw_event/->event -- and
those interfaces demonstrate that a bind with msc->input == NULL is a
state real devices reach, not only spoofed ones. Happy to re-test a v2
on the same hardware over both USB and Bluetooth.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] HID: magicmouse: reject devices that bind without an input device
2026-07-28 7:25 ` Alec Hall
@ 2026-07-28 18:37 ` Jose Villaseñor Montfort
0 siblings, 0 replies; 5+ messages in thread
From: Jose Villaseñor Montfort @ 2026-07-28 18:37 UTC (permalink / raw)
To: Alec Hall
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
Jose Villaseñor Montfort
On Tue, Jul 28, 2026, Alec Hall wrote:
> The second half of that turns out not to hold on real hardware. A USB
> Magic Trackpad 2 exposes four HID interfaces, and only the first two
> register an input; interfaces 2 and 3 are vendor-defined, hiddev/hidraw
> only.
Thanks for putting it on hardware. You're right, and the stable argument
settles it -- unbinding two interfaces of a healthy device is not
something to ship as a fix. v1 is dropped.
I can confirm the same shape on the other model. A USB-C Magic Trackpad
(05ac:0324) on an unpatched 7.1.5 here exposes three HID interfaces
rather than four, all three bound to magicmouse:
0003:05AC:0324.0006 input=input12 hidraw=hidraw5
0003:05AC:0324.0007 input=input13 hidraw=hidraw6
0003:05AC:0324.0008 input=<none> hidraw=hidraw7
Interface 2 is the vendor-defined one (bInterfaceSubClass 0,
bInterfaceProtocol 0, two endpoints) and has no input device. So the
interface count varies by model, but an input-less interface bound to
this driver is plain normal.
One thing worth adding to the picture: those interfaces do not merely
bind with msc->input == NULL, they can also reach the dereference.
hid_process_event() calls ->event without checking HID_CLAIMED_INPUT:
if (hdrv && hdrv->event && hid_match_usage(hid, usage)) {
ret = hdrv->event(hid, field, usage, value);
...
}
if (hid->claimed & HID_CLAIMED_INPUT)
hidinput_hid_event(hid, field, usage, value);
and hid_match_usage() returns 1 for everything here, since magicmouse
declares no usage_table. So an input report on interface 2 or 3 walks
into magicmouse_event(), which dereferences msc->input->id.product right
at the top; ->raw_event is called for every report regardless. The NULL
deref is therefore reachable on genuine hardware, not only on a device
spoofing an Apple VID/PID -- a better justification than the one I sent,
so thanks for that as well.
v2 with the per-callback guards follows shortly. Both callbacks return 0,
so those reports keep flowing through the generic HID paths exactly as
they do today, and magicmouse_probe() is left alone. I've credited the
change of approach to you with Suggested-by.
The offer to re-test over USB and Bluetooth is very welcome, thank you.
Jose
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] HID: magicmouse: avoid NULL pointer deref when there is no input device
2026-07-15 19:58 [PATCH] HID: magicmouse: reject devices that bind without an input device Jose Villaseñor Montfort
2026-07-28 7:25 ` Alec Hall
@ 2026-07-28 18:40 ` Jose Villaseñor Montfort
2026-07-28 18:55 ` sashiko-bot
1 sibling, 1 reply; 5+ messages in thread
From: Jose Villaseñor Montfort @ 2026-07-28 18:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Alec Hall, linux-input, linux-kernel,
Jose Villaseñor Montfort
magicmouse_raw_event() and magicmouse_event() dereference msc->input
(e.g. input->id.product, and via magicmouse_emit_touch() and
magicmouse_emit_buttons()) without checking it for NULL. hid-input only
sets msc->input when the device exposes a usable input device.
magicmouse_probe() has an "input not registered" check that fails the
probe when msc->input is NULL, but the USB Magic Mouse 2 / Magic
Trackpad 2 path returns 0 before reaching it. Real hardware ends up in
that state: a USB Magic Trackpad 2 exposes four HID interfaces and only
the first two register an input, and a USB-C Magic Trackpad (05ac:0324)
exposes three with the same split. The remaining interfaces are
vendor-defined, are claimed as hiddev/hidraw only, and stay bound to
this driver with msc->input == NULL.
Both callbacks are reachable in that state. ->raw_event is called for
every incoming report, and hid_process_event() calls ->event without
requiring HID_CLAIMED_INPUT -- hid_match_usage() matches everything here
because this driver has no usage_table. An input report on one of those
interfaces therefore dereferences a NULL pointer and panics the kernel.
A device that binds this driver by spoofing an Apple VID/PID, with a
report descriptor that does not produce an input device, reaches the
same state.
Bail out of both callbacks when msc->input is NULL and leave the report
to the generic HID paths, which is what those interfaces get today.
Rejecting the bind in magicmouse_probe() instead would unbind interfaces
that a healthy device legitimately exposes and drop their hidraw nodes.
Fixes: 0b91b4e4dae6 ("HID: magicmouse: Report battery level over USB")
Suggested-by: Alec Hall <signshop.alec@gmail.com>
Link: https://lore.kernel.org/linux-input/20260714102540.3EB2E1F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260728072554.47069-1-signshop.alec@gmail.com/
Cc: stable@vger.kernel.org
Signed-off-by: Jose Villaseñor Montfort <pepemontfort@gmail.com>
---
Changes in v2:
- Switch from rejecting the bind in magicmouse_probe() to guarding both
callbacks. Alec Hall tested v1 on a USB Magic Trackpad 2 and found that
it unbinds interfaces 2 and 3: they are vendor-defined, have no input
device, and have been claimed by this driver since 0b91b4e4dae6, so
every cable plug logged two -ENOMEM probe failures and their hidraw
nodes went away. Nothing user-facing broke (touch and battery live on
interfaces 0/1), but silently unbinding a healthy device's interfaces
is not a change to make in a stable-tagged fix. Report:
https://lore.kernel.org/linux-input/20260728072554.47069-1-signshop.alec@gmail.com/
- I confirmed the same shape on a USB-C Magic Trackpad (05ac:0324) here,
on an unpatched 7.1.5: three HID interfaces, all three bound to
magicmouse, and the last one -- vendor-defined, bInterfaceSubClass 0,
two endpoints -- has a hidraw node but no input device. So the exact
interface count varies by model, but at least one input-less interface
bound to this driver is normal on genuine hardware.
- Reworded the commit message accordingly. Those interfaces show that a
bind with msc->input == NULL is a state real hardware reaches, not only
a spoofed device; and hid_process_event() calls ->event regardless of
HID_CLAIMED_INPUT (hid_match_usage() matches everything, this driver
has no usage_table), so ->event is reachable on them too.
- v1: https://lore.kernel.org/linux-input/20260715195853.1302765-1-pepemontfort@gmail.com/
This is a sibling hardening fix to "HID: magicmouse: prevent unbounded
recursion in magicmouse_raw_event()" [1], which touches the same driver.
If that one is applied first, this needs a trivial rebase: the guard
moves into __magicmouse_raw_event().
[1] https://lore.kernel.org/linux-input/20260715053526.574725-1-pepemontfort@gmail.com/
drivers/hid/hid-magicmouse.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 802a3479e..7164ced59 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -390,6 +390,14 @@ static int magicmouse_raw_event(struct hid_device *hdev,
struct input_dev *input = msc->input;
int x = 0, y = 0, ii, clicks = 0, npoints;
+ /*
+ * This driver also binds interfaces that hid-input does not create an
+ * input device for, so msc->input can legitimately be NULL here. Leave
+ * their reports to the generic HID paths instead of dereferencing it.
+ */
+ if (!input)
+ return 0;
+
/* Protect against zero sized recursive calls from DOUBLE_REPORT_ID */
if (size < 1)
return 0;
@@ -538,6 +546,11 @@ static int magicmouse_event(struct hid_device *hdev, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{
struct magicmouse_sc *msc = hid_get_drvdata(hdev);
+
+ /* See the comment in magicmouse_raw_event(). */
+ if (!msc->input)
+ return 0;
+
if ((msc->input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 ||
msc->input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE2_USBC) &&
field->report->id == MOUSE2_REPORT_ID) {
base-commit: b7556c8e713c88596046a906c7c4385218d44736
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] HID: magicmouse: avoid NULL pointer deref when there is no input device
2026-07-28 18:40 ` [PATCH v2] HID: magicmouse: avoid NULL pointer deref when there is no " Jose Villaseñor Montfort
@ 2026-07-28 18:55 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-28 18:55 UTC (permalink / raw)
To: Jose Villaseñor Montfort; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Unbounded recursion in `magicmouse_raw_event()` when processing `DOUBLE_REPORT_ID` leads to kernel stack overflow.
- [High] Dangling `msc->input` pointer leading to Use-After-Free if `input_register_device()` fails.
--
--- Patch [1]: [PATCH v2] HID: magicmouse: avoid NULL pointer deref when there is no input device ---
commit 43afb4a20d87a9959f50f2b092097b19eab03cdd
Author: Jose Villaseñor Montfort <pepemontfort@gmail.com>
HID: magicmouse: avoid NULL pointer deref when there is no input device
This patch guards against NULL pointer dereferences in the raw_event and event
callbacks for the magicmouse driver. It bails out if the input device is NULL,
which is a legitimate state for some hardware interfaces that do not register
an input device but still bind to the driver.
Link: https://lore.kernel.org/linux-input/20260714102540.3EB2E1F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260728072554.47069-1-signshop.alec@gmail.com/
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 802a3479e24b9..7164ced590764 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -390,6 +390,14 @@ static int magicmouse_raw_event(struct hid_device *hdev,
> struct input_dev *input = msc->input;
> int x = 0, y = 0, ii, clicks = 0, npoints;
>
> + /*
> + * This driver also binds interfaces that hid-input does not create an
> + * input device for, so msc->input can legitimately be NULL here. Leave
> + * their reports to the generic HID paths instead of dereferencing it.
> + */
> + if (!input)
> + return 0;
> +
[Severity: High]
This isn't a bug introduced by this patch, but does the !input check miss
dangling pointers if input_register_device() fails?
During device initialization, hid_hw_start() calls hidinput_connect(). This
function allocates an input_dev and invokes the driver's input_mapping
callback, which caches the pointer:
drivers/hid/hid-magicmouse.c:magicmouse_input_mapping() {
...
if (!msc->input)
msc->input = hi->input;
...
}
If input_register_device() subsequently fails, the HID core frees the
input_dev but does not notify the driver, leaving msc->input dangling.
magicmouse_probe() can then succeed without realizing the input device was
freed, as it either returns early or evaluates the dangling pointer as true:
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) {
hid_err(hdev, "magicmouse input not registered\n");
...
}
When a raw HID report arrives, would this freed pointer bypass the !input
guard and be dereferenced?
> /* Protect against zero sized recursive calls from DOUBLE_REPORT_ID */
> if (size < 1)
> return 0;
[Severity: Critical]
This is a pre-existing issue, but does this function have an unbounded
recursion bug when processing a DOUBLE_REPORT_ID?
If data[1] is 0, the first recursive call returns immediately:
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;
...
}
However, the second call will process the remainder of the buffer, reducing
the size by only 2 bytes per iteration. With a maximum-sized HID report, could
this cause up to ~8192 recursive calls, exhausting the standard 16KB kernel
stack and leading to a kernel panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728184059.688513-1-pepemontfort@gmail.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-28 18:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 19:58 [PATCH] HID: magicmouse: reject devices that bind without an input device Jose Villaseñor Montfort
2026-07-28 7:25 ` Alec Hall
2026-07-28 18:37 ` Jose Villaseñor Montfort
2026-07-28 18:40 ` [PATCH v2] HID: magicmouse: avoid NULL pointer deref when there is no " Jose Villaseñor Montfort
2026-07-28 18:55 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.