LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v3 1/1] gpio: mockup: Convert to use software nodes
@ 2021-10-05 12:24 Andy Shevchenko
  2021-10-06 11:18 ` Bartosz Golaszewski
  0 siblings, 1 reply; 2+ messages in thread
From: Andy Shevchenko @ 2021-10-05 12:24 UTC (permalink / raw)
  To: Andy Shevchenko, linux-gpio, linux-kernel
  Cc: Bamvor Jian Zhang, Linus Walleij, Bartosz Golaszewski, Kent Gibson

The gpio-mockup driver creates the 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 a use-after-free
as shown in the following 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 the lifetime of the software nodes
by itself.

The problem originates from the old device_add_properties() API, but
has been only revealed after the commit bd1e336aa853 ("driver core: platform:
Remove platform_device_add_properties()"). Hence, it's used as a landmark
for backporting.

Fixes: bd1e336aa853 ("driver core: platform: Remove platform_device_add_properties()")
Reported-and-tested-by: Kent Gibson <warthog618@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: fixed tag, more grammar fixes
 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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v3 1/1] gpio: mockup: Convert to use software nodes
  2021-10-05 12:24 [PATCH v3 1/1] gpio: mockup: Convert to use software nodes Andy Shevchenko
@ 2021-10-06 11:18 ` Bartosz Golaszewski
  0 siblings, 0 replies; 2+ messages in thread
From: Bartosz Golaszewski @ 2021-10-06 11:18 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
	Bamvor Jian Zhang, Linus Walleij, Kent Gibson

On Tue, Oct 5, 2021 at 2:24 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> The gpio-mockup driver creates the 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 a use-after-free
> as shown in the following 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 the lifetime of the software nodes
> by itself.
>
> The problem originates from the old device_add_properties() API, but
> has been only revealed after the commit bd1e336aa853 ("driver core: platform:
> Remove platform_device_add_properties()"). Hence, it's used as a landmark
> for backporting.
>
> Fixes: bd1e336aa853 ("driver core: platform: Remove platform_device_add_properties()")
> Reported-and-tested-by: Kent Gibson <warthog618@gmail.com>

I prefer to have the Reported-by and Tested-by tags separately even
for the same person. I fixed that in my tree.

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v3: fixed tag, more grammar fixes
>  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
>

Applied for fixes, thanks!

Bart

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-10-06 11:18 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 12:24 [PATCH v3 1/1] gpio: mockup: Convert to use software nodes Andy Shevchenko
2021-10-06 11:18 ` Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).