mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-10-01 06:33:07 +00:00
gpiolib: fix memory leak in gpiochip_setup_dev()
Here is a backtrace report about memory leak detected in
gpiochip_setup_dev():
unreferenced object 0xffff88810b406400 (size 512):
comm "python3", pid 1682, jiffies 4295346908 (age 24.090s)
backtrace:
kmalloc_trace
device_add device_private_init at drivers/base/core.c:3361
(inlined by) device_add at drivers/base/core.c:3411
cdev_device_add
gpiolib_cdev_register
gpiochip_setup_dev
gpiochip_add_data_with_key
gcdev_register() & gcdev_unregister() would call device_add() &
device_del() (no matter CONFIG_GPIO_CDEV is enabled or not) to
register/unregister device.
However, if device_add() succeeds, some resource (like
struct device_private allocated by device_private_init())
is not released by device_del().
Therefore, after device_add() succeeds by gcdev_register(), it
needs to call put_device() to release resource in the error handle
path.
Here we move forward the register of release function, and let it
release every piece of resource by put_device() instead of kfree().
While at it, fix another subtle issue, i.e. when gc->ngpio is equal
to 0, we still call kcalloc() and, in case of further error, kfree()
on the ZERO_PTR pointer, which is not NULL. It's not a bug per se,
but rather waste of the resources and potentially wrong expectation
about contents of the gdev->descs variable.
Fixes: 159f3cd92f
("gpiolib: Defer gpio device setup until after gpiolib initialization")
Signed-off-by: Zeng Heng <zengheng4@huawei.com>
Co-developed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
This commit is contained in:
parent
b7b275e60b
commit
ec851b2308
1 changed files with 26 additions and 16 deletions
|
@ -526,12 +526,13 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
|
||||||
if (ret)
|
if (ret)
|
||||||
return ret;
|
return ret;
|
||||||
|
|
||||||
|
/* From this point, the .release() function cleans up gpio_device */
|
||||||
|
gdev->dev.release = gpiodevice_release;
|
||||||
|
|
||||||
ret = gpiochip_sysfs_register(gdev);
|
ret = gpiochip_sysfs_register(gdev);
|
||||||
if (ret)
|
if (ret)
|
||||||
goto err_remove_device;
|
goto err_remove_device;
|
||||||
|
|
||||||
/* From this point, the .release() function cleans up gpio_device */
|
|
||||||
gdev->dev.release = gpiodevice_release;
|
|
||||||
dev_dbg(&gdev->dev, "registered GPIOs %d to %d on %s\n", gdev->base,
|
dev_dbg(&gdev->dev, "registered GPIOs %d to %d on %s\n", gdev->base,
|
||||||
gdev->base + gdev->ngpio - 1, gdev->chip->label ? : "generic");
|
gdev->base + gdev->ngpio - 1, gdev->chip->label ? : "generic");
|
||||||
|
|
||||||
|
@ -597,10 +598,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
|
||||||
struct fwnode_handle *fwnode = NULL;
|
struct fwnode_handle *fwnode = NULL;
|
||||||
struct gpio_device *gdev;
|
struct gpio_device *gdev;
|
||||||
unsigned long flags;
|
unsigned long flags;
|
||||||
int base = gc->base;
|
|
||||||
unsigned int i;
|
unsigned int i;
|
||||||
|
u32 ngpios = 0;
|
||||||
|
int base = 0;
|
||||||
int ret = 0;
|
int ret = 0;
|
||||||
u32 ngpios;
|
|
||||||
|
|
||||||
if (gc->fwnode)
|
if (gc->fwnode)
|
||||||
fwnode = gc->fwnode;
|
fwnode = gc->fwnode;
|
||||||
|
@ -647,17 +648,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
|
||||||
else
|
else
|
||||||
gdev->owner = THIS_MODULE;
|
gdev->owner = THIS_MODULE;
|
||||||
|
|
||||||
gdev->descs = kcalloc(gc->ngpio, sizeof(gdev->descs[0]), GFP_KERNEL);
|
|
||||||
if (!gdev->descs) {
|
|
||||||
ret = -ENOMEM;
|
|
||||||
goto err_free_dev_name;
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Try the device properties if the driver didn't supply the number
|
* Try the device properties if the driver didn't supply the number
|
||||||
* of GPIO lines.
|
* of GPIO lines.
|
||||||
*/
|
*/
|
||||||
if (gc->ngpio == 0) {
|
ngpios = gc->ngpio;
|
||||||
|
if (ngpios == 0) {
|
||||||
ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios);
|
ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios);
|
||||||
if (ret == -ENODATA)
|
if (ret == -ENODATA)
|
||||||
/*
|
/*
|
||||||
|
@ -668,7 +664,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
|
||||||
*/
|
*/
|
||||||
ngpios = 0;
|
ngpios = 0;
|
||||||
else if (ret)
|
else if (ret)
|
||||||
goto err_free_descs;
|
goto err_free_dev_name;
|
||||||
|
|
||||||
gc->ngpio = ngpios;
|
gc->ngpio = ngpios;
|
||||||
}
|
}
|
||||||
|
@ -676,13 +672,19 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
|
||||||
if (gc->ngpio == 0) {
|
if (gc->ngpio == 0) {
|
||||||
chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
|
chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
|
||||||
ret = -EINVAL;
|
ret = -EINVAL;
|
||||||
goto err_free_descs;
|
goto err_free_dev_name;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (gc->ngpio > FASTPATH_NGPIO)
|
if (gc->ngpio > FASTPATH_NGPIO)
|
||||||
chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
|
chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
|
||||||
gc->ngpio, FASTPATH_NGPIO);
|
gc->ngpio, FASTPATH_NGPIO);
|
||||||
|
|
||||||
|
gdev->descs = kcalloc(gc->ngpio, sizeof(*gdev->descs), GFP_KERNEL);
|
||||||
|
if (!gdev->descs) {
|
||||||
|
ret = -ENOMEM;
|
||||||
|
goto err_free_dev_name;
|
||||||
|
}
|
||||||
|
|
||||||
gdev->label = kstrdup_const(gc->label ?: "unknown", GFP_KERNEL);
|
gdev->label = kstrdup_const(gc->label ?: "unknown", GFP_KERNEL);
|
||||||
if (!gdev->label) {
|
if (!gdev->label) {
|
||||||
ret = -ENOMEM;
|
ret = -ENOMEM;
|
||||||
|
@ -701,11 +703,13 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
|
||||||
* it may be a pipe dream. It will not happen before we get rid
|
* it may be a pipe dream. It will not happen before we get rid
|
||||||
* of the sysfs interface anyways.
|
* of the sysfs interface anyways.
|
||||||
*/
|
*/
|
||||||
|
base = gc->base;
|
||||||
if (base < 0) {
|
if (base < 0) {
|
||||||
base = gpiochip_find_base(gc->ngpio);
|
base = gpiochip_find_base(gc->ngpio);
|
||||||
if (base < 0) {
|
if (base < 0) {
|
||||||
ret = base;
|
|
||||||
spin_unlock_irqrestore(&gpio_lock, flags);
|
spin_unlock_irqrestore(&gpio_lock, flags);
|
||||||
|
ret = base;
|
||||||
|
base = 0;
|
||||||
goto err_free_label;
|
goto err_free_label;
|
||||||
}
|
}
|
||||||
/*
|
/*
|
||||||
|
@ -816,6 +820,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
|
||||||
err_free_gpiochip_mask:
|
err_free_gpiochip_mask:
|
||||||
gpiochip_remove_pin_ranges(gc);
|
gpiochip_remove_pin_ranges(gc);
|
||||||
gpiochip_free_valid_mask(gc);
|
gpiochip_free_valid_mask(gc);
|
||||||
|
if (gdev->dev.release) {
|
||||||
|
/* release() has been registered by gpiochip_setup_dev() */
|
||||||
|
put_device(&gdev->dev);
|
||||||
|
goto err_print_message;
|
||||||
|
}
|
||||||
err_remove_from_list:
|
err_remove_from_list:
|
||||||
spin_lock_irqsave(&gpio_lock, flags);
|
spin_lock_irqsave(&gpio_lock, flags);
|
||||||
list_del(&gdev->list);
|
list_del(&gdev->list);
|
||||||
|
@ -829,13 +838,14 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
|
||||||
err_free_ida:
|
err_free_ida:
|
||||||
ida_free(&gpio_ida, gdev->id);
|
ida_free(&gpio_ida, gdev->id);
|
||||||
err_free_gdev:
|
err_free_gdev:
|
||||||
|
kfree(gdev);
|
||||||
|
err_print_message:
|
||||||
/* failures here can mean systems won't boot... */
|
/* failures here can mean systems won't boot... */
|
||||||
if (ret != -EPROBE_DEFER) {
|
if (ret != -EPROBE_DEFER) {
|
||||||
pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", __func__,
|
pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", __func__,
|
||||||
gdev->base, gdev->base + gdev->ngpio - 1,
|
base, base + (int)ngpios - 1,
|
||||||
gc->label ? : "generic", ret);
|
gc->label ? : "generic", ret);
|
||||||
}
|
}
|
||||||
kfree(gdev);
|
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(gpiochip_add_data_with_key);
|
EXPORT_SYMBOL_GPL(gpiochip_add_data_with_key);
|
||||||
|
|
Loading…
Reference in a new issue