Re: [PATCH v6 04/13] HID: asus: add gamepad configuration
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-08 18:01:08
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Time-of-check to time-of-use race condition in probe allows spoofed USB devices to bypass singleton checks, leading to memory corruption and UAF. - [Medium] Sysfs attribute becomes visible before device initialization is complete, causing transient -ENODEV errors. -- commit 40a018a18704e85c526230c53db4138c9ec858c7 Author: Denis Benato [off-list ref] HID: asus: add gamepad configuration Add the base configuration structures for the gamepad configuration, detect capabilities and initialize the device in a known state.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index 11dea2aec80b1..ac0c0a176857e 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -617,6 +660,274 @@ static u8 *ally_alloc_cmd(u8 cmd, const u8 *payload, u8 payload_size) return hidbuf; }
[ ... ]
quoted hunk ↗ jump to hunk
+static ssize_t xbox_controller_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct hid_device *hdev = to_hid_device(dev); + struct asus_drvdata *drvdata = hid_get_drvdata(hdev); + struct ally_handheld *ally = drvdata->rog_ally; + struct ally_config *cfg; + + if (!ally) + return -ENODEV;
[Severity: Medium] Will userspace configuration scripts encounter transient errors when trying to read or configure the controller immediately upon detection? In ally_config_create(), sysfs_create_group() is called, which immediately exposes attributes like xbox_controller to userspace. If a read or write occurs immediately, xbox_controller_show() retrieves drvdata->rog_ally. However, this pointer is only assigned later in asus_probe() after hid_asus_ally_probe() returns. During this window, this callback will return -ENODEV. While this race is handled later in the series for button mapping attributes by explicitly carrying the ally pointer in a custom attribute structure, do these base attributes need similar protection?
quoted hunk ↗ jump to hunk
+ + cfg = ally_get_config(ally); + if (!cfg) + return -ENODEV;
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1025,6 +1356,14 @@ static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev) return ERR_PTR(ret); } + ally_cfg = ally_config_create(hdev, &ally_drvdata);
[Severity: High] Can this code allow a time-of-check to time-of-use race if multiple USB interfaces are probed concurrently? The singleton check earlier in hid_asus_ally_probe() checks ally_drvdata.cfg_hdev under ally_data_lock, but the lock is released before reaching this point. If two threads probe concurrently, both will pass the initial check. They would then both execute ally_config_create() concurrently without a lock, leading to a memory leak of the static ally_config pointer and a double initialization of its mutex. Finally, if both threads re-acquire the lock and overwrite ally_drvdata.cfg_hdev, the device that loses the race would have its sysfs groups permanently leaked. If the driver is later unloaded, ally_config is freed, but the leaked sysfs attributes would remain, leading to a use-after-free on subsequent accesses.
quoted hunk ↗ jump to hunk
+ if (IS_ERR(ally_cfg)) { + hid_err(hdev, "Failed to create Ally cfg: %ld\n", + PTR_ERR(ally_cfg)); + ally_put_udev_if_orphaned(); + return ERR_PTR(PTR_ERR(ally_cfg)); + } + ret = hid_asus_ally_init(hdev, &ally_drvdata);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260908174433.74260-1-denis.benato@linux.dev?part=4