Re: [PATCH 2/4] HID: hid-msi-claw: Add M-key mapping attributes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Data race on `drvdata->profile_pending` and lack of transaction atomicity during concurrent sysfs accesses.
--

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

HID: hid-msi-claw: Add M-key mapping attributes

Adds sysfs attributes to remap the M1 and M2 rear buttons to up to 5 events
(gamepad, keyboard, or mouse) when in macro mode.

> diff --git a/drivers/hid/hid-msi-claw.c b/drivers/hid/hid-msi-claw.c
> index 7a3cd940ec49e..60694d075d56f 100644
> --- a/drivers/hid/hid-msi-claw.c
> +++ b/drivers/hid/hid-msi-claw.c
[ ... ]
> +static int claw_buttons_store(struct device *dev, const char *buf, u8 mkey_idx)
> +{
[ ... ]
> +set_buttons:
> +	ret = mcu_property_out(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA, data, len, 8);
> +	if (ret < 0)
> +		goto err_free;
> +
> +	ret = mcu_property_out(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);

Is it possible for concurrent writers to interleave these commands?

Looking at mcu_property_out(), it takes the cfg_mutex internally, but there
is no lock held across both calls here. If a second writer executes its
WRITE_PROFILE_DATA before the first writer executes SYNC_TO_ROM, could the MCU
be left in an inconsistent state?

[ ... ]
> +static int claw_buttons_show(struct device *dev, char *buf, enum claw_key_index m_key)
> +{
[ ... ]
> +	codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
> +	drvdata->profile_pending = (m_key == CLAW_KEY_M1) ? CLAW_M1_PENDING : CLAW_M2_PENDING;
> +
> +	ret = mcu_property_out(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data, len, 8);

Can concurrent reads of button_m1 and button_m2 race and corrupt the
profile_pending state?

If Thread A and Thread B access the sysfs attributes simultaneously,
profile_pending is updated locklessly here before mcu_property_out() takes
the cfg_mutex.

Could Thread B overwrite profile_pending to CLAW_M2_PENDING before Thread A's
command executes? If so, when the hardware replies to Thread A's M1 command,
claw_profile_event() might route Thread A's data into Thread B's m2_codes
buffer and clear the state to CLAW_NO_PENDING.

This would cause Thread B's subsequent reply to be dropped and trigger the
"Got profile event without changes pending" warning.

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