Re: [PATCH v5 05/12] Input: xbox_gip - Add battery support
From: sashiko-bot@kernel.org
Date: 2026-09-02 01:20:39
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Devres actions on `input->dev` are delayed until userspace drops all references, causing the power supply to outlive the `attachment` structure and resulting in a Use-After-Free. - [High] Manually unregistering a devm-managed power supply and freeing its name on an error path leads to a double-unregister and UAF when devres cleanup executes. -- commit 7a0e19630adf51b35545780bfe01529fdc2adba8 Author: Vicki Pfau [off-list ref] Input: xbox_gip - Add battery support This commit adds battery support to the xbox_gip driver by exposing the controller's battery information as a power_supply device.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/joystick/gip/gip-core.c b/drivers/input/joystick/gip/gip-core.c --- a/drivers/input/joystick/gip/gip-core.c +++ b/drivers/input/joystick/gip/gip-core.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1199,6 +1206,106 @@ static int gip_guide_led_probe(struct gip_attachment *attachment, struct device
[ ... ]
+static int gip_battery_create(struct gip_attachment *attachment, struct device *dev)
+{
+ struct power_supply_config supply_config = { .drv_data = attachment, };
+ int rc;[ ... ]
+ attachment->battery = devm_power_supply_register(dev,
+ &attachment->battery_desc, &supply_config);
+ if (IS_ERR(attachment->battery)) {
+ rc = PTR_ERR(attachment->battery);
+ goto err_out;
+ }
+
+ rc = power_supply_powers(attachment->battery, dev);
+ if (rc < 0) {
+ power_supply_unregister(attachment->battery);
+ goto err_out;
+ }[Severity: High] Because attachment->battery was allocated using devm_power_supply_register(), a devres action is already queued to unregister it. If power_supply_powers() fails and we manually call power_supply_unregister() here, will devres attempt to unregister it again later when the input device is cleaned up, leading to a double-free? [ ... ]
quoted hunk ↗ jump to hunk
@@ -1494,6 +1604,8 @@ static void gip_free_devices(struct gip_attachment *attachment) rcu_assign_pointer(attachment->input, NULL); rcu_assign_pointer(attachment->hdev, NULL); + /* The following are freed by devres */ + attachment->battery = NULL; synchronize_rcu();
[Severity: High] The battery was registered with devm using the input device (input->dev). Since input device devres actions are delayed until userspace drops all evdev file descriptor references, does setting attachment->battery to NULL here leave the power supply active after the driver disconnects? If a user reads the sysfs attributes for the power supply after the device is unplugged, could gip_battery_get_property() attempt to access the freed attachment structure? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260902010237.865772-1-vi@endrift.com?part=5