Re: [PATCH v2] HID: magicmouse: avoid NULL pointer deref when there is no input device

[Jose Villaseñor Montfort]

On Tue, Jul 28, 2026, sashiko-bot@xxxxxxxxxx wrote:
> [Severity: High]
> This isn't a bug introduced by this patch, but does the !input check miss
> dangling pointers if input_register_device() fails?

It does, and I checked the path: this looks like a real bug, separate
from the one this patch fixes. Thanks.

hidinput_connect() unwinds through hidinput_disconnect() when
input_register_device() fails, and that frees every input_dev it created
-- including the one magicmouse_input_mapping() cached in msc->input.

What makes it reachable is that 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 right below does not fire for
this driver, which sets ->raw_event (and on the USB paths hidraw/hiddev
are claimed anyway). So hid_hw_start() returns 0, magicmouse_probe()
carries on, and msc->input is non-NULL garbage: it passes the "input not
registered" check in probe just as it passes the NULL guards this patch
adds. The next report dereferences freed memory.

The v1 of this patch, which rejected the bind in probe(), would not have
covered it either, for the same reason.

I will send a separate patch for it rather than fold it in here, since
it is a different failure (use-after-free on an error path, not a NULL
deref on a normal bind) and this one already has a Fixes: tag of its own.
The fix I have in mind is to trust the core's claim rather than the
cached pointer, i.e. clear msc->input in probe when the HID core did not
claim an input device, so the existing NULL checks cover this case too.
I am open to a different shape if reviewers prefer one.

> [Severity: Critical]
> This is a pre-existing issue, but does this function have an unbounded
> recursion bug when processing a DOUBLE_REPORT_ID?

Yes, and that one is already fixed in a patch on the list, with
Reviewed-by and Tested-by from Alec Hall:

  https://lore.kernel.org/linux-input/20260715053526.574725-1-pepemontfort@xxxxxxxxx/

It bounds the depth to two levels by rejecting a nested DOUBLE_REPORT_ID,
since a double report never wraps another one.

Jose