Re: [PATCH v3 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:
- [High] The asynchronous request state machine lacks sequence tracking and proper synchronization, causing delayed USB events to misroute and corrupt data for subsequent operations.
--

commit 95ec4f316951f2159554c90bdf669eb45a19e099
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-msi: Add Rumble Intensity Attributes

This commit adds sysfs attributes to control the left and right rumble motor
intensity on MSI Claw devices. It implements store and show functions that
communicate with the device firmware and uses a state machine to handle the
asynchronous responses.

> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 06dc4290d2bb5..349a59f75e471 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c

[ ... ]

> @@ -810,6 +826,126 @@ static ssize_t button_mapping_options_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(button_mapping_options);
>  

[ ... ]

> +static ssize_t rumble_intensity_left_show(struct device *dev,
> +					  struct device_attribute *attr,
> +					  char *buf)
> +{
> +	u8 data[4] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01 };
> +	struct hid_device *hdev = to_hid_device(dev);
> +	struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> +	int ret;
> +
> +	guard(mutex)(&drvdata->profile_mutex);
> +	drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
> +	ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
> +				    ARRAY_SIZE(data), 8);
> +	if (ret) {
> +		drvdata->profile_pending = CLAW_NO_PENDING;
> +		return ret;
> +	}

Is the state machine vulnerable to crossed data from delayed USB events?

If claw_hw_output_report() times out (using the 8ms timeout), this function
clears profile_pending and returns. If a subsequent read for the right
motor then sets profile_pending to CLAW_RUMBLE_RIGHT_PENDING, could a
delayed response from this left motor request arrive?

If so, claw_profile_event() would see CLAW_RUMBLE_RIGHT_PENDING,
incorrectly save the left rumble data into drvdata->rumble_intensity_right,
and clear the pending state. This would complete the right rumble request
with the left motor's data.

Similarly, could a stray ACK from a timed-out store operation prematurely
complete a show operation, exiting without clearing profile_pending and
permanently corrupting the state machine?

> +
> +	return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_left);
> +}

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