Re: [PATCH v5 13/13] HID: asus: add support for btn remapping
From: sashiko-bot@kernel.org
Date: 2026-09-04 15:46:21
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] A NULL pointer dereference occurs in `ally_set_button_mapping` during initialization on devices that lack turbo support, leading to a kernel panic.
- [Medium] New sysfs attributes are exposed to userspace before the driver's private data is fully initialized, causing spurious -ENODEV errors on early reads.
--
commit 555564cf1146bdf536948c12422013a569b55b96
Author: Denis Benato [off-list ref]
HID: asus: add support for btn remapping
This commit introduces button remapping for the ROG Ally, allowing
custom mapping for gamepad, keyboard, mouse, and media codes. It adds
new sysfs attributes for configuring the physical buttons based on the
current controller mode.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index 7c10e6de438b7..a8b1d407863d7 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1034,6 +1077,295 @@ static int ally_set_gamepad_mode(struct ally_handheld *ally, struct hid_device * +static int ally_set_button_mapping(struct hid_device *hdev, struct ally_handheld *ally, + struct button_pair_map *mapping) +{ + /* The MCU mapping block is four consecutive 11-byte entries starting at + * buf[5]: first remap 5-15, first macro 16-26, second remap 27-37, + * second macro 38-48 (see hid-asus-ally __btn_pair_to_pkt, BTN_CODE_LEN). + */ + u8 macro_bytes[11] = {0}; + u8 btn_bytes[11] = {0}; + + if (!mapping) + return -EINVAL; + + u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_MAPPING, NULL, 0); + if (!buf) + return -ENOMEM; + + /* This packet is slightly different from the other + * as before the packet length there is an extra byte + * which is the pair index. + */ + buf[3] = mapping->pair_index; + buf[4] = 0x2C; /* Length */ + + /* First button mapping */ + buf[5] = mapping->first.remap->type;
[Severity: High] If a device lacks turbo support, the caller skips initializing the button attributes, which leaves cfg->button_mappings zero-initialized. When hid_asus_ally_init() unconditionally calls ally_apply_button_mappings(), does this result in a NULL pointer dereference here when accessing mapping->first.remap->type?
+ /* Fill in bytes 6-14 with button code */
+ if (mapping->first.remap->type) {[ ... ]
quoted hunk ↗ jump to hunk
@@ -3714,6 +4681,8 @@ static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *all +static ssize_t button_remap_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;
[Severity: Medium] Can userspace trigger a spurious -ENODEV error here on early reads? The sysfs files are created during hid_asus_ally_probe(), exposing them to userspace immediately. However, drvdata->rog_ally is only assigned in asus_probe() after hid_asus_ally_probe() returns. If a userspace daemon reads this file in that window, drvdata->rog_ally will be NULL. The older attributes avoided this race by caching the ally pointer in struct ally_btn_sysfs_entry.
+ struct button_remap_attr *btn_attr = to_button_remap_attr(attr); + const struct btn_code_map *code; + struct ally_config *cfg; + enum ally_button_id button_id = btn_attr->button_id; + enum btn_pair_index pair_idx; + struct button_pair_map *pair; + struct button_map *btn_map; + bool is_first; + int ret; + + if (!ally) + return -ENODEV;
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260904145845.184887-1-denis.benato@linux.dev?part=13