Re: [PATCH v5 12/13] HID: asus: add support for turbo buttons
From: sashiko-bot@kernel.org
Date: 2026-09-04 16:02:32
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing cleanup of user calibration sysfs groups (`ally_cal_attr_groups`) in the probe error path of `ally_config_create()`.
--
commit 64475826d55426435b32a4fe5bb92d505b7ed1b7
Author: Denis Benato [off-list ref]
HID: asus: add support for turbo buttons
ROG ally devices supports a feature called turbo buttons that allows
the user to quickly toggle button state by keeping a button pressed.
This commit allows userspace to configure the feature by exposing
relevant sysfs attributes.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index 63efda8938386..7c10e6de438b7 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2697,6 +3126,14 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al } } + if (cfg->turbo_support) { + ret = ally_create_button_attributes(hdev, ally, cfg); + if (ret < 0) { + hid_err(hdev, "Failed to create button attributes: %d\n", ret); + goto ally_config_create_sysfs_err;
[Severity: High] If ally_create_button_attributes() fails here, does this jump leak the sysfs groups that were previously created for ally_cal_attr_groups? This newly added code introduces a failure point after ally_cal_attr_groups has already been successfully created, making the missing cleanup reachable.
quoted hunk ↗ jump to hunk
+ } + } + /* So far the only hardware this is supported is the Ally 1 */ if (cfg->xbox_controller_support) { ret = ally_set_xbox_controller(hdev, ally, cfg, true);@@ -2717,6 +3154,9 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al return cfg; ally_config_create_sysfs_err: + if (cfg->button_entries) + ally_remove_button_attributes(hdev, cfg); +
[Severity: High] Does this error recovery path completely omit the sysfs_remove_group() cleanup loop for ally_cal_attr_groups? Leaking these sysfs group directories and attributes can cause subsequent attempts to probe the device to trigger a kernel warning for duplicate sysfs nodes (-EEXIST). This would render the device permanently unusable without a reboot.
/* The sysfs groups are not devm-managed: undo the created ones. */ while (sysfs_i-- > 0) sysfs_remove_group(&hdev->dev.kobj, &ally_attr_groups[sysfs_i]);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260904145845.184887-1-denis.benato@linux.dev?part=12