Thread (4 messages) flat view 4 messages, 2 authors, 2d ago

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

From: sashiko-bot@kernel.org
Date: 2026-09-12 02:08:56

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help