Re: [PATCH] HID: magicmouse: add haptic click configuration for Magic Trackpad 2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The module parameter `haptic_click` is read multiple times without being cached, resulting in a Time-of-Check to Time-of-Use (TOCTOU) race condition.
--

commit 160ed71822c4a0697b90debaa037251a7e50642c
Author: Christian Fressl <christian@fressl.at>

HID: magicmouse: add haptic click configuration for Magic Trackpad 2

This commit adds support for configuring the haptic feedback on Apple
Magic Trackpad 2 devices. It introduces the haptic_click module parameter
to choose between unchanged, silent-low, or off states.

Link: https://github.com/mwyborski/Linux-Magic-Trackpad-2-Driver/issues/28#issuecomment-451625504

> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index e70bd3dc07ab7..31002cb1f1089 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c

[ ... ]

> @@ -812,6 +827,81 @@ static bool is_usb_magictrackpad2(__u32 vendor, __u32 product)

[ ... ]

> +static int magicmouse_apply_haptic_click(struct hid_device *hdev)
> +{
> +	u32 click_feedback;
> +	u32 release_feedback;
> +	int ret;
> +
> +	if (haptic_click == TRACKPAD2_HAPTIC_CLICK_UNCHANGED)
> +		return 0;
> +
> +	switch (haptic_click) {

Since haptic_click is a module parameter with 0644 permissions, can it
change concurrently between the if statement and the switch statement?

If it changes from TRACKPAD2_HAPTIC_CLICK_SILENT_LOW (1) to
TRACKPAD2_HAPTIC_CLICK_UNCHANGED (0) right after the early return check,
this regression could cause the switch statement to fall through to the
default case. This would result in an incorrect warning and return -EINVAL.

Would it be safer to read the parameter into a local variable once using
READ_ONCE() to avoid this race condition?

> +	case TRACKPAD2_HAPTIC_CLICK_SILENT_LOW:
> +		click_feedback = TRACKPAD2_HAPTIC_SILENT_CLICK;
> +		release_feedback = TRACKPAD2_HAPTIC_SILENT_RELEASE;
> +		break;
> +	case TRACKPAD2_HAPTIC_CLICK_OFF:
> +		click_feedback = TRACKPAD2_HAPTIC_OFF;
> +		release_feedback = TRACKPAD2_HAPTIC_OFF;
> +		break;
> +	default:
> +		hid_warn(hdev, "invalid haptic_click value %u\n", haptic_click);
> +		return -EINVAL;
> +	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511203115.104087-1-christian@fressl.at?part=1