[PATCH] HID: magicmouse: do not keep a stale msc->input if no input is claimed

[PATCH] HID: magicmouse: do not keep a stale msc->input if no input is claimed

[Jose Villaseñor Montfort]

magicmouse_input_mapping() caches the first hid_input's input_dev in
msc->input while the report descriptor is parsed, and the rest of the
driver treats a non-NULL msc->input as proof that an input device was
registered.

That does not hold on the hid-input error path. If hidinput_connect()
fails -- for instance because input_register_device() returns an error --
it unwinds through hidinput_disconnect(), which frees every input_dev it
created, including the one cached in msc->input.

The failure does not abort the probe. hid_connect() only skips the claim:

	if ((connect_mask & HID_CONNECT_HIDINPUT) && !hidinput_connect(hdev,
				connect_mask & HID_CONNECT_HIDINPUT_FORCE))
		hdev->claimed |= HID_CLAIMED_INPUT;

and the "device has no listeners" bailout below it does not fire for this
driver, which sets ->raw_event; on the USB Magic Mouse 2 / Magic Trackpad
2 paths hidraw and hiddev are claimed as well. hid_hw_start() therefore
returns 0 and magicmouse_probe() continues with msc->input pointing at
freed memory. Being non-NULL, it passes the "input not registered" check
in probe and the NULL checks in ->raw_event and ->event, so the next
input report dereferences freed memory.

Clear msc->input when the HID core did not claim an input device, so the
existing NULL checks cover this case as well.

Fixes: f1a9a149abc8 ("HID: magicmouse: fix race between input_register() and probe()")
Link: https://lore.kernel.org/linux-input/20260728185542.65F091F000E9@smtp.kernel.org/
Cc: stable@vger.kernel.org
Signed-off-by: Jose Villaseñor Montfort <pepemontfort@gmail.com>
---
Found by the Sashiko AI review of "[PATCH v2] HID: magicmouse: avoid NULL
pointer deref when there is no input device" (Link: above); I verified
the path before writing this. That patch [1] is a sibling fix that adds
the NULL checks in ->raw_event and ->event which this one makes
effective on the error path; it applies independently of this one.

I picked f1a9a149abc8 for the Fixes: tag because that is where probe()
started treating a non-NULL msc->input as proof that an input device was
registered. The dangling pointer itself is older than that; happy to
change the tag if reviewers prefer a different one, or to drop the
stable Cc, given this needs an input_register_device() failure to
trigger.

Also a sibling to "HID: magicmouse: prevent unbounded recursion in
magicmouse_raw_event()" [2], which touches the same driver but a
different function.

[1] https://lore.kernel.org/linux-input/20260728184059.688513-1-pepemontfort@gmail.com/
[2] https://lore.kernel.org/linux-input/20260715053526.574725-1-pepemontfort@gmail.com/

 drivers/hid/hid-magicmouse.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 802a3479e..2f14094a6 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -900,6 +900,16 @@ static int magicmouse_probe(struct hid_device *hdev,
 		return ret;
 	}
 
+	/*
+	 * When hidinput_connect() fails it frees every input device it
+	 * created, but that does not fail hid_hw_start(): the core simply
+	 * does not claim an input. msc->input, cached in ->input_mapping
+	 * while the report descriptor was parsed, would then be a dangling
+	 * pointer that passes every NULL check. Trust the core's claim.
+	 */
+	if (!(hdev->claimed & HID_CLAIMED_INPUT))
+		msc->input = NULL;
+
 	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);

base-commit: b7556c8e713c88596046a906c7c4385218d44736
-- 
2.55.0

Re: [PATCH] HID: magicmouse: do not keep a stale msc->input if no input is claimed

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Deterministic NULL pointer dereference in event handlers after a failed input registration.
--

--- Patch [1]: [PATCH] HID: magicmouse: do not keep a stale msc->input if no input is claimed ---
commit 6fea999082f28bca408072a6788f3616b0ebbd9f
Author: Jose Villaseñor Montfort <pepemontfort@gmail.com>

HID: magicmouse: do not keep a stale msc->input if no input is claimed

This patch clears msc->input when the HID core does not claim an input
device. This is intended to ensure existing NULL checks safely handle the
failure path where hidinput_connect() frees the input devices.

Link: https://lore.kernel.org/linux-input/20260728185542.65F091F000E9@smtp.kernel.org/

> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 802a3479e24b9..2f14094a63b1e 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -900,6 +900,16 @@ static int magicmouse_probe(struct hid_device *hdev,
>  		return ret;
>  	}
>  
> +	/*
> +	 * When hidinput_connect() fails it frees every input device it
> +	 * created, but that does not fail hid_hw_start(): the core simply
> +	 * does not claim an input. msc->input, cached in ->input_mapping
> +	 * while the report descriptor was parsed, would then be a dangling
> +	 * pointer that passes every NULL check. Trust the core's claim.
> +	 */
> +	if (!(hdev->claimed & HID_CLAIMED_INPUT))
> +		msc->input = NULL;
> +

[Severity: High]
If msc->input is set to NULL here due to a failure, could this cause a NULL
pointer dereference for certain USB devices?

Looking further down in magicmouse_probe(), there is an early return on
success for specific devices that bypasses the failure check:

drivers/hid/hid-magicmouse.c:magicmouse_probe() {
    ...
    if (!(hdev->claimed & HID_CLAIMED_INPUT))
        msc->input = NULL;
    ...
    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");
        ...
}

If the probe succeeds by returning 0, the driver remains bound.

When an input report is later received, will magicmouse_raw_event() and
magicmouse_event() unconditionally dereference the NULL msc->input pointer?

For example, looking at magicmouse_raw_event():

drivers/hid/hid-magicmouse.c:magicmouse_raw_event() {
    ...
    struct input_dev *input = msc->input;
    ...
    if (input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE ||
    ...
}

And similarly in magicmouse_event():

drivers/hid/hid-magicmouse.c:magicmouse_event() {
    ...
    struct magicmouse_sc *msc = hid_get_drvdata(hdev);
    if ((msc->input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 ||
    ...
}

>  	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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729041557.1185819-1-pepemontfort@gmail.com?part=1