Re: [PATCH 5/5] HID: steam: Zero out inputs when disabling gamepad mode

[sashiko-bot@kernel.org]

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

New issues:
- [High] AB-BA deadlock between `report_mutex` and workqueues during device unregistration.

Pre-existing issues:
- [High] Concurrent unregistration in `steam_probe` error path leads to a double free of input devices.
--

--- Patch [5]: [PATCH 5/5] HID: steam: Zero out inputs when disabling gamepad mode ---
commit 7a64f604e0a34949b1434a03544f0ca71ef9305f
Author: Vicki Pfau <vi@endrift.com>

HID: steam: Zero out inputs when disabling gamepad mode

When gamepad mode is disabled the gamepad input devices will stop receiving
updates. However, in the case where there are buttons still pressed this
will appear as an indefinitely-held button. Instead we should zero out the
inputs to make it look like things are all released.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 87af4bee9f5ff..93a6f4e17fb75 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
> @@ -1490,11 +1490,76 @@ static void steam_mode_switch_cb(struct work_struct *work)
>  
>  	guard(mutex)(&steam->report_mutex);

[Severity: High]
Is there a risk of an AB-BA deadlock here during device unregistration?

If steam_remove() is called and acquires steam->report_mutex, it then calls
steam_unregister(). If steam_unregister() then calls
cancel_delayed_work_sync(&steam->mode_switch) while the mutex is held:

steam_remove()
  guard(mutex)(&steam->report_mutex);
  steam_unregister(steam);
    cancel_delayed_work_sync(&steam->mode_switch);

This workqueue callback would block here attempting to acquire the same
steam->report_mutex, while steam_remove() blocks waiting for this work
to finish. Since the wait condition is permanent, this could hang the USB/HID
hotplug subsystem.

[ ... ]

[Severity: High]
This is a pre-existing issue, but could the error path in steam_probe() race
with disconnect events and cause a double-free of the input devices?

If steam_probe() fails in hid_add_device(), it jumps to err_steam_unregister
and calls steam_unregister() if steam->connected is true (checked locklessly):

steam_probe():
err_steam_unregister:
	if (steam->connected)
		steam_unregister(steam);

Because hid_device_io_start() was already called, a concurrent disconnect
event can queue steam_work_connect_cb(). If that worker runs, sees
connected == false, and also calls steam_unregister(steam), both threads can
concurrently enter steam_input_unregister() since the error path does not
hold report_mutex.

Both threads would then read steam->input, find it valid, set it to NULL
locklessly, and call input_unregister_device() on the same pointer:

steam_input_unregister():
	input = rcu_dereference(steam->input);
	...
	RCU_INIT_POINTER(steam->input, NULL);
	...
	input_unregister_device(input);

Can this lead to a double-free of the input_dev structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805015116.3884465-1-vi@endrift.com?part=5