Re: [PATCH v11 4/4] HID: hid-msi: Add Rumble Intensity Attributes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unwaited ACKs from SYNC_TO_ROM commands can prematurely complete the waits of subsequent unrelated commands.
--

commit 11fcab6387a146514df018948771932cc7f57ed2
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-msi: Add Rumble Intensity Attributes

This commit adds intensity adjustment attributes for the left and right
rumble motors on MSI Claw devices.

> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 96710d80543d4..f5c54cd62f3bb 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -930,6 +962,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
[ ... ]
> +static ssize_t rumble_intensity_left_store(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t count)
> +{
[ ... ]
> +	ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> +				    (u8 *)&report, sizeof(report), 25);
> +	if (ret)
> +		return ret;
> +
> +	/* MCU will not send ACK until the USB transaction completes. ACK is sent
> +	 * immediately after and will hit the stale state machine, before the next
> +	 * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
> +	 * for ACK that ill never come.
> +	 */
> +	ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);

[Severity: High]
This is a pre-existing issue, but this patch propagates it to the new rumble
attributes.

If a SYNC_TO_ROM command is sent with a timeout of 0, the driver does not
wait for the hardware's ACK, but the hardware still sends a generic ACK
packet for it.

If userspace rapidly writes to multiple sysfs attributes (for example,
rumble_intensity_left then rumble_intensity_right), the second write will
send a WRITE_PROFILE_DATA command and wait for its ACK. Can the stray ACK
from the first SYNC_TO_ROM command arrive during this wait window?

Since the generic ACK doesn't indicate which command it belongs to, would
this cause the driver to falsely assume the WRITE_PROFILE_DATA command has
completed and prematurely wake the waiter? This could cause the driver to
send the next command while the hardware is still processing the previous
one, leading to dropped commands and device state desynchronization.

> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529072111.7565-1-derekjohn.clark@gmail.com?part=4