Re: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth

[Alec Hall <signshop.alec@gmail.com>]

> 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