Re: [PATCH v4 1/2] HID: flydigi: add support for vader 5 pro

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Use-after-free of custom workqueue due to devres lifecycle mismatch.
- [High] Invalid pointer dereference of `ERR_PTR` on probe failure.
--

commit 7c54f2d9cfb04d2dfe81eebdf0f885848becc9b4
Author: Denis Benato <denis.benato@linux.dev>

HID: flydigi: add support for vader 5 pro

This commit introduces support for the Flydigi Vader Pro 5 gamepad. It
exposes two evdev nodes with matching unique IDs for the gamepad and motion
sensors so userspace applications can pair them.

> diff --git a/drivers/hid/hid-flydigi.c b/drivers/hid/hid-flydigi.c
> new file mode 100644
> index 0000000000000..148c53564200f
> --- /dev/null
> +++ b/drivers/hid/hid-flydigi.c

[ ... ]

> +	fd->battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
> +	fd->battery_capacity = 0;
> +
> +	fd->battery = devm_power_supply_register(&hdev->dev, &fd->battery_desc,
> +						 &psy_cfg);
> +	if (IS_ERR(fd->battery))
> +		return PTR_ERR(fd->battery);

[Severity: High]
Can this result in an invalid pointer dereference on probe failure?

If devm_power_supply_register() fails, fd->battery is directly assigned an
ERR_PTR. The function returns an error, leading to the err_stop cleanup path.

If an incoming FLYDIGI_CMD_INFO packet from the device is processed during
this cleanup window (before hid_hw_stop() fully disables events), it will
call flydigi_report_battery():

    if (fd->battery)
        power_supply_changed(fd->battery);

Since an ERR_PTR evaluates to true, the error pointer will be passed to
power_supply_changed() and dereferenced. This regression could be triggered
by an untrusted USB device spamming status packets.

[ ... ]

> +static void flydigi_remove(struct hid_device *hdev)
> +{
> +	struct flydigi_device *fd = hid_get_drvdata(hdev);
> +
> +	scoped_guard(spinlock_irqsave, &fd->lock) {
> +		fd->removed = true;
> +		fd->strong = 0;
> +		fd->weak = 0;
> +	}
> +
> +	cancel_delayed_work_sync(&fd->acquire_work);
> +	cancel_delayed_work_sync(&fd->rumble_work);
> +	destroy_workqueue(fd->wq);

[Severity: High]
Does this cause a use-after-free of the workqueue due to a devres lifecycle
mismatch?

The custom workqueue fd->wq is explicitly destroyed here in flydigi_remove()
and similarly in the err_stop probe error path. However, the input devices
are allocated with devm_input_allocate_device(), meaning they will be
unregistered by devres after flydigi_remove() completes.

During this window, the input devices are still visible to userspace. If
userspace opens the gamepad or sends force-feedback effects, callbacks like
flydigi_gamepad_open() or flydigi_play_effect() will attempt to queue work
on the destroyed fd->wq, leading to a use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912015926.230874-1-denis.benato@linux.dev?part=1