Re: [PATCH v7 5/5] gpio: shared: convert to using software nodes for GPIO shared proxy lookup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpio: shared: use-after-free in gpio_shared_remove_proxy_dev() on swnode teardown
--

commit 8040c150f4a36d793af070d87bfa19010fce092d
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

gpio: shared: convert to using software nodes for GPIO shared proxy lookup

This commit replaces platform lookup tables for shared GPIO proxy devices
with dummy software nodes assigned to the proxy auxiliary devices. This
allows looking up the target proxy GPIO device by its firmware node even
when no consumer struct device is present.

> diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
> index c0e2a1207cccf..4f5ecdfcc63ac 100644
> --- a/drivers/gpio/gpiolib-shared.c
> +++ b/drivers/gpio/gpiolib-shared.c

[ ... ]

> @@ -529,6 +540,8 @@ static void gpio_shared_remove_proxy_dev(struct gpio_shared_ref *ref)
>  	scoped_guard(mutex, &ref->lock) {
>  		proxy = ref->proxy_dev;
>  		ref->proxy_dev = NULL;
> +		software_node_unregister(&ref->proxy_swnode);
> +		ref->proxy_fwnode = NULL;
>  	}

[Severity: High]
Does unregistering ref->proxy_swnode before deleting the proxy auxiliary
device cause a use-after-free of the struct swnode during device removal?

In gpio_shared_make_proxy_dev(), software_node_register() allocates the
struct swnode with an initial reference count of 1 (and swnode->managed
set to false), and device_set_node() attaches its fwnode to adev->dev.
When auxiliary_device_add() registers adev->dev (and when the proxy driver
registers its gpio_device), software_node_notify() increments the swnode
reference count for each device.

In gpio_shared_remove_proxy_dev(), calling software_node_unregister()
before auxiliary_device_delete(proxy) drops the initial creation reference
while proxy->dev is still registered:

gpio_shared_remove_proxy_dev()
  software_node_unregister(&ref->proxy_swnode) -> drops initial ref
  auxiliary_device_delete(proxy)
    device_del(&proxy->dev)
      device_platform_notify_remove(&proxy->dev)
        software_node_notify_remove(&proxy->dev)

When software_node_notify_remove() runs in drivers/base/swnode.c,
swnode_put() drops the last remaining reference and frees the struct
swnode, and then immediately reads from the freed structure:

	swnode_put(swnode);

	if (swnode->managed) {
		set_secondary_fwnode(dev, NULL);
		swnode_put(swnode);
	}

In addition, because swnode->managed is false, proxy->dev.fwnode is not
cleared, so acpi_device_notify_remove() in device_platform_notify_remove()
immediately dereferences dev->fwnode->ops on the freed swnode.

Should software_node_unregister(&ref->proxy_swnode) be called after
auxiliary_device_delete(proxy) and auxiliary_device_uninit(proxy) (or with
device_set_node(&proxy->dev, NULL) before unregistering), matching the
error unwind order in gpio_shared_make_proxy_dev()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-gpio-shared-fwnode-lookup-v7-0-acb453b6fbd5@oss.qualcomm.com?part=5