* [PATCH v1 1/1] gpio: mockup: Convert to use software nodes
@ 2021-10-05 9:10 Andy Shevchenko
2021-10-05 9:27 ` Kent Gibson
2021-10-05 9:34 ` Bartosz Golaszewski
0 siblings, 2 replies; 5+ messages in thread
From: Andy Shevchenko @ 2021-10-05 9:10 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 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")
Reported-by: Kent Gibson <warthog618@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
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 related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] gpio: mockup: Convert to use software nodes
2021-10-05 9:10 [PATCH v1 1/1] gpio: mockup: Convert to use software nodes Andy Shevchenko
@ 2021-10-05 9:27 ` Kent Gibson
2021-10-05 9:30 ` Andy Shevchenko
2021-10-05 9:34 ` Bartosz Golaszewski
1 sibling, 1 reply; 5+ messages in thread
From: Kent Gibson @ 2021-10-05 9:27 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-gpio, linux-kernel, Bamvor Jian Zhang, Linus Walleij,
Bartosz Golaszewski
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@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Other than that, looks good and works for me.
Tested-by: Kent Gibson <warthog618@gmail.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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] gpio: mockup: Convert to use software nodes
2021-10-05 9:27 ` Kent Gibson
@ 2021-10-05 9:30 ` Andy Shevchenko
0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2021-10-05 9:30 UTC (permalink / raw)
To: Kent Gibson
Cc: Andy Shevchenko, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, Bamvor Jian Zhang, Linus Walleij,
Bartosz Golaszewski
On Tue, Oct 5, 2021 at 12:28 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> 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()")
I think you are right. I thought that it happened during the last rc-week.
> > Reported-by: Kent Gibson <warthog618@gmail.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Other than that, looks good and works for me.
>
> Tested-by: Kent Gibson <warthog618@gmail.com>
Thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] gpio: mockup: Convert to use software nodes
2021-10-05 9:10 [PATCH v1 1/1] gpio: mockup: Convert to use software nodes Andy Shevchenko
2021-10-05 9:27 ` Kent Gibson
@ 2021-10-05 9:34 ` Bartosz Golaszewski
2021-10-05 9:41 ` Andy Shevchenko
1 sibling, 1 reply; 5+ messages in thread
From: Bartosz Golaszewski @ 2021-10-05 9:34 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 11:10 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> 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")
> Reported-by: Kent Gibson <warthog618@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> 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
>
I suppose my gpio-sim patch is affected by this issue too. Thanks for
fixing that.
Bart
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] gpio: mockup: Convert to use software nodes
2021-10-05 9:34 ` Bartosz Golaszewski
@ 2021-10-05 9:41 ` Andy Shevchenko
0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2021-10-05 9:41 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andy Shevchenko, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, Bamvor Jian Zhang, Linus Walleij,
Kent Gibson
On Tue, Oct 5, 2021 at 12:35 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Tue, Oct 5, 2021 at 11:10 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> I suppose my gpio-sim patch is affected by this issue too. Thanks for
> fixing that.
As far as I remember the code (it was pretty much inherited from
gpio-mockup) yes, it's also affected.
Btw, I just sent a v2.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-05 9:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 9:10 [PATCH v1 1/1] gpio: mockup: Convert to use software nodes Andy Shevchenko
2021-10-05 9:27 ` Kent Gibson
2021-10-05 9:30 ` Andy Shevchenko
2021-10-05 9:34 ` Bartosz Golaszewski
2021-10-05 9:41 ` Andy Shevchenko
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).