linux-kernel - Re: [PATCH v1 1/1] gpio: mockup: Convert to use software nodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211005092753.GA911482@sol>
Date:   Tue, 5 Oct 2021 17:27:53 +0800
From:   Kent Gibson <warthog618@...il.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bamvor Jian Zhang <bamv2005@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>
Subject: Re: [PATCH v1 1/1] gpio: mockup: Convert to use software nodes

On Tue, Oct 05, 2021 at 12:10:16PM +0300, Andy Shevchenko wrote:
> The gpio-mockup driver creates a properties that are shared between
> platform and GPIO devices. Because of that, the properties may not
> be removed at the proper point of time without provoking use-after-free
> as shown in the backtrace:
> 
>   refcount_t: underflow; use-after-free.
>   WARNING: CPU: 0 PID: 103 at lib/refcount.c:28 refcount_warn_saturate+0xd1/0x120
>   ...
>   Call Trace:
>   kobject_put+0xdc/0xf0
>   software_node_notify_remove+0xa8/0xc0
>   device_del+0x15a/0x3e0
> 
> That's why the driver has to manage lifetime of the software nodes by itself.
> 
> The problem originates by the old device_add_properties() API, but has been
> only revealed after the commit 5aeb05b27f81 ("software node: balance refcount
> for managed software nodes"). Hence, it's used as landmark for the backporting.
> 
> Fixes: 5aeb05b27f81 ("software node: balance refcount for managed software nodes")

Shouldn't that be:
Fixes: bd1e336aa853 ("driver core: platform: Remove platform_device_add_properties()")

> Reported-by: Kent Gibson <warthog618@...il.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>

Other than that, looks good and works for me.

Tested-by: Kent Gibson <warthog618@...il.com>

Cheers,
Kent.

> ---
>  drivers/gpio/gpio-mockup.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> index 0a9d746a0fe0..8b147b565e92 100644
> --- a/drivers/gpio/gpio-mockup.c
> +++ b/drivers/gpio/gpio-mockup.c
> @@ -478,8 +478,18 @@ static void gpio_mockup_unregister_pdevs(void)
>  {
>  	int i;
>  
> -	for (i = 0; i < GPIO_MOCKUP_MAX_GC; i++)
> -		platform_device_unregister(gpio_mockup_pdevs[i]);
> +	for (i = 0; i < GPIO_MOCKUP_MAX_GC; i++) {
> +		struct platform_device *pdev;
> +		struct fwnode_handle *fwnode;
> +
> +		pdev = gpio_mockup_pdevs[i];
> +		if (!pdev)
> +			continue;
> +
> +		fwnode = dev_fwnode(&pdev->dev);
> +		platform_device_unregister(pdev);
> +		fwnode_remove_software_node(fwnode);
> +	}
>  }
>  
>  static __init char **gpio_mockup_make_line_names(const char *label,
> @@ -508,6 +518,7 @@ static int __init gpio_mockup_register_chip(int idx)
>  	struct property_entry properties[GPIO_MOCKUP_MAX_PROP];
>  	struct platform_device_info pdevinfo;
>  	struct platform_device *pdev;
> +	struct fwnode_handle *fwnode;
>  	char **line_names = NULL;
>  	char chip_label[32];
>  	int prop = 0, base;
> @@ -536,13 +547,18 @@ static int __init gpio_mockup_register_chip(int idx)
>  					"gpio-line-names", line_names, ngpio);
>  	}
>  
> +	fwnode = fwnode_create_software_node(properties, NULL);
> +	if (IS_ERR(fwnode))
> +		return PTR_ERR(fwnode);
> +
>  	pdevinfo.name = "gpio-mockup";
>  	pdevinfo.id = idx;
> -	pdevinfo.properties = properties;
> +	pdevinfo.fwnode = fwnode;
>  
>  	pdev = platform_device_register_full(&pdevinfo);
>  	kfree_strarray(line_names, ngpio);
>  	if (IS_ERR(pdev)) {
> +		fwnode_remove_software_node(fwnode);
>  		pr_err("error registering device");
>  		return PTR_ERR(pdev);
>  	}
> -- 
> 2.33.0
>