Commit aa1e2bfd authored by Zeng Heng's avatar Zeng Heng Committed by He Yujie
Browse files

gpiolib: fix memory leak in gpiochip_setup_dev() 

mainline inclusion
from mainline-v6.1
commit ec851b23
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYRIL
CVE: CVE-2022-48975

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ec851b23084b3a0af8bf0f5e51d33a8d678bdc49



--------------------------------

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: 159f3cd9 ("gpiolib: Defer gpio device setup until after gpiolib initialization")
Signed-off-by: default avatarZeng Heng <zengheng4@huawei.com>
Co-developed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: default avatarBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Conflicts:
	drivers/gpio/gpiolib.c
[ 3cc1fb73(gpiolib: do not print err message for EPROBE_DEFER).
  990f6756(gpiolib: allow to specify the firmware node in struct gpio_chip).
  9dbd1ab2(gpiolib: check the 'ngpios' property in core gpiolib code).
  context conflicts because the preceding three commit commands are not merge]
Signed-off-by: default avatarHe Yujie <coka.heyujie@huawei.com>
parent 170b758d

drivers/gpio/gpiolib.c

+22 −13
Original line number Diff line number Diff line
@@ -500,12 +500,13 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) 
	if (ret)

		return ret;



	/* From this point, the .release() function cleans up gpio_device */

	gdev->dev.release = gpiodevice_release;



	ret = gpiochip_sysfs_register(gdev);

	if (ret)

		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,

		gdev->base + gdev->ngpio - 1, gdev->chip->label ? : "generic");
@@ -572,7 +573,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 
	unsigned long	flags;

	int		ret = 0;

	unsigned	i;

	int		base = gc->base;

	int		base = 0;

	struct gpio_device *gdev;



	/*
@@ -624,22 +625,22 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 
	else

		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;

	}



	if (gc->ngpio == 0) {

		chip_err(gc, "tried to insert a GPIO chip with zero lines\n");

		ret = -EINVAL;

		goto err_free_descs;

		goto err_free_dev_name;

	}



	if (gc->ngpio > FASTPATH_NGPIO)

		chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",

			  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);

	if (!gdev->label) {

		ret = -ENOMEM;
@@ -658,11 +659,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

	 * of the sysfs interface anyways.

	 */

	base = gc->base;

	if (base < 0) {

		base = gpiochip_find_base(gc->ngpio);

		if (base < 0) {

			ret = base;

			spin_unlock_irqrestore(&gpio_lock, flags);

			ret = base;

			base = 0;

			goto err_free_label;

		}

		/*
@@ -770,6 +773,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 
	of_gpiochip_remove(gc);

err_free_gpiochip_mask:

	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:

	spin_lock_irqsave(&gpio_lock, flags);

	list_del(&gdev->list);
@@ -783,11 +791,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 
err_free_ida:

	ida_free(&gpio_ida, gdev->id);

err_free_gdev:

	kfree(gdev);

err_print_message:

	/* failures here can mean systems won't boot... */

	pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", __func__,

	       gdev->base, gdev->base + gdev->ngpio - 1,

	       base, base + gdev->ngpio - 1,

	       gc->label ? : "generic", ret);

	kfree(gdev);

	return ret;

}

EXPORT_SYMBOL_GPL(gpiochip_add_data_with_key);