| Message ID | 20211005122449.16296-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/1] gpio: mockup: Convert to use software nodes | expand |
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
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); }
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(-)