Re: [STABLE v5.15-v5.10 6/6] HID: steam: Clean up locking

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hid: steam: TOCTOU race in input_open/close corrupts lizard mode
--

commit ad9df30996ddcb8a183a74dda605518707156658
Author: Vicki Pfau <vi@endrift.com>
Date:   Tue Oct 13 09:12:13 2026 +0000

    HID: steam: Clean up locking
    
    This cleans up several issues with locking behavior, including RCU accesses
    not being guarded behind a lock.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 5414f446753de..df21c48928303 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -284,9 +285,14 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
>  static int steam_input_open(struct input_dev *dev)
>  {
>  	struct steam_device *steam = input_get_drvdata(dev);
> +	unsigned long flags;
> +	bool client_opened;
>  
> +	spin_lock_irqsave(&steam->lock, flags);
> +	client_opened = steam->client_opened;
> +	spin_unlock_irqrestore(&steam->lock, flags);
>  	mutex_lock(&steam->mutex);
> -	if (!steam->client_opened && lizard_mode)
> +	if (!client_opened && lizard_mode)
>  		steam_set_lizard_mode(steam, false);
>  	mutex_unlock(&steam->mutex);
>  	return 0;

[Severity: Medium]
Could this introduce a TOCTOU race condition? We read steam->client_opened 
under steam->lock into a local variable, drop the lock, and then block on 
steam->mutex. 

Concurrently, a raw hidraw client can open or close the device, which modifies
steam->client_opened under steam->lock. Does this allow the function to act
on stale state while holding the mutex and leave lizard mode incorrectly enabled?

>  }
> @@ -295,9 +301,14 @@ static int steam_input_open(struct input_dev *dev)
>  static void steam_input_close(struct input_dev *dev)
>  {
>  	struct steam_device *steam = input_get_drvdata(dev);
> +	unsigned long flags;
> +	bool client_opened;
>  
> +	spin_lock_irqsave(&steam->lock, flags);
> +	client_opened = steam->client_opened;
> +	spin_unlock_irqrestore(&steam->lock, flags);
>  	mutex_lock(&steam->mutex);
> -	if (!steam->client_opened && lizard_mode)
> +	if (!client_opened && lizard_mode)
>  		steam_set_lizard_mode(steam, true);
>  	mutex_unlock(&steam->mutex);
>  }

[Severity: Medium]
Similar to steam_input_open(), does this TOCTOU window allow
steam_input_close() to incorrectly turn lizard mode ON just as a concurrent
raw hidraw client is opening the device and attempting to manage it? 

Since the Steam client only sends its initialization commands (such as
disabling lizard mode) once upon startup and does not continuously poll, it
seems it would not fix a spurious state change forced by the driver behind
its back.

[ ... ]
> @@ -1098,6 +1113,8 @@ static int steam_param_set_lizard_mode(const char *val,
>  {
>  	struct steam_device *steam;
>  	int ret;
> +	bool client_opened;
> +	unsigned long flags;
>  
>  	ret = param_set_bool(val, kp);
>  	if (ret)
> @@ -1105,10 +1122,14 @@ static int steam_param_set_lizard_mode(const char *val,
>  
>  	mutex_lock(&steam_devices_lock);
>  	list_for_each_entry(steam, &steam_devices, list) {
> -		mutex_lock(&steam->mutex);
> -		if (!steam->client_opened)
> +		spin_lock_irqsave(&steam->lock, flags);
> +		client_opened = steam->client_opened;
> +		spin_unlock_irqrestore(&steam->lock, flags);
> +		if (!client_opened) {
> +			mutex_lock(&steam->mutex);
>  			steam_set_lizard_mode(steam, lizard_mode);
> -		mutex_unlock(&steam->mutex);
> +			mutex_unlock(&steam->mutex);
> +		}
>  	}
>  	mutex_unlock(&steam_devices_lock);
>  	return 0;

[Severity: Medium]
Does this have the same TOCTOU race with steam->client_opened as 
steam_input_open() and steam_input_close() when setting the lizard mode
parameter via sysfs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013091215.529531-1-lee@kernel.org?part=6