Patch "gpio: sysfs: fix memory leaks and device hotplug" has been added to the 3.14-stable tree

Patch "gpio: sysfs: fix memory leaks and device hotplug" has been added to the 3.14-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    gpio: sysfs: fix memory leaks and device hotplug

to the 3.14-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     gpio-sysfs-fix-memory-leaks-and-device-hotplug.patch
and it can be found in the queue-3.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From 483d821108791092798f5d230686868112927044 Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan@kernel.org>
Date: Tue, 21 Apr 2015 17:42:09 +0200
Subject: gpio: sysfs: fix memory leaks and device hotplug

From: Johan Hovold <johan@kernel.org>

commit 483d821108791092798f5d230686868112927044 upstream.

Unregister GPIOs requested through sysfs at chip remove to avoid leaking
the associated memory and sysfs entries.

The stale sysfs entries prevented the gpio numbers from being exported
when the gpio range was later reused (e.g. at device reconnect).

This also fixes the related module-reference leak.

Note that kernfs makes sure that any on-going sysfs operations finish
before the class devices are unregistered and that further accesses
fail.

The chip exported flag is used to prevent gpiod exports during removal.
This also makes it harder to trigger, but does not fix, the related race
between gpiochip_remove and export_store, which is really a race with
gpiod_request that needs to be addressed separately.

Also note that this would prevent the crashes (e.g. NULL-dereferences)
at reconnect that affects pre-3.18 kernels, as well as use-after-free on
operations on open attribute files on pre-3.14 kernels (prior to
kernfs).

Fixes: d8f388d8dc8d ("gpio: sysfs interface")
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/gpio/gpiolib.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -800,6 +800,7 @@ static struct class gpio_class = {
  */
 int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 {
+	struct gpio_chip	*chip;
 	unsigned long		flags;
 	int			status;
 	const char		*ioname = NULL;
@@ -817,8 +818,16 @@ int gpiod_export(struct gpio_desc *desc,
 		return -EINVAL;
 	}
 
+	chip = desc->chip;
+
 	mutex_lock(&sysfs_lock);
 
+	/* check if chip is being removed */
+	if (!chip || !chip->exported) {
+		status = -ENODEV;
+		goto fail_unlock;
+	}
+
 	spin_lock_irqsave(&gpio_lock, flags);
 	if (!test_bit(FLAG_REQUESTED, &desc->flags) ||
 	     test_bit(FLAG_EXPORT, &desc->flags)) {
@@ -1057,12 +1066,15 @@ static void gpiochip_unexport(struct gpi
 {
 	int			status;
 	struct device		*dev;
+	struct gpio_desc *desc;
+	unsigned int i;
 
 	mutex_lock(&sysfs_lock);
 	dev = class_find_device(&gpio_class, NULL, chip, match_export);
 	if (dev) {
 		put_device(dev);
 		device_unregister(dev);
+		/* prevent further gpiod exports */
 		chip->exported = false;
 		status = 0;
 	} else
@@ -1071,6 +1083,13 @@ static void gpiochip_unexport(struct gpi
 
 	if (status)
 		chip_dbg(chip, "%s: status %d\n", __func__, status);
+
+	/* unregister gpiod class devices owned by sysfs */
+	for (i = 0; i < chip->ngpio; i++) {
+		desc = &chip->desc[i];
+		if (test_and_clear_bit(FLAG_SYSFS, &desc->flags))
+			gpiod_free(desc);
+	}
 }
 
 static int __init gpiolib_sysfs_init(void)


Patches currently in stable-queue which might be from johan@kernel.org are

queue-3.14/gpio-sysfs-fix-memory-leaks-and-device-hotplug.patch

Re: Patch "gpio: sysfs: fix memory leaks and device hotplug" has been added to the 3.14-stable tree

[Johan Hovold]

On Thu, May 14, 2015 at 06:57:31PM -0700, Greg Kroah-Hartman wrote:
> 
> This is a note to let you know that I've just added the patch titled
> 
>     gpio: sysfs: fix memory leaks and device hotplug
> 
> to the 3.14-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      gpio-sysfs-fix-memory-leaks-and-device-hotplug.patch
> and it can be found in the queue-3.14 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.

Your backport looks good, but this one depends on commit 01cca93a9491
("gpio: unregister gpiochip device before removing it") which needs to
be backported as well. I tried to convey this in the stable tag:

	Cc: stable <stable@vger.kernel.org> # v2.6.27: 01cca93a9491

I'll respond to this message with a tested backport of both patches.

Thanks,
Johan

[PATCH-3.14 1/2] gpio: unregister gpiochip device before removing it

[Johan Hovold]

commit 01cca93a9491ed95992523ff7e79dd9bfcdea8e0 upstream.

Unregister gpiochip device (used to export information through sysfs)
before removing it internally. This way removal will reverse addition.

Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f1fc14c33be5..7fc186c6927d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1265,6 +1265,8 @@ int gpiochip_remove(struct gpio_chip *chip)
 	int		status = 0;
 	unsigned	id;
 
+	gpiochip_unexport(chip);
+
 	spin_lock_irqsave(&gpio_lock, flags);
 
 	gpiochip_remove_pin_ranges(chip);
@@ -1286,9 +1288,6 @@ int gpiochip_remove(struct gpio_chip *chip)
 
 	spin_unlock_irqrestore(&gpio_lock, flags);
 
-	if (status == 0)
-		gpiochip_unexport(chip);
-
 	return status;
 }
 EXPORT_SYMBOL_GPL(gpiochip_remove);
-- 
2.3.6

[PATCH-3.14 2/2] gpio: sysfs: fix memory leaks and device hotplug

[Johan Hovold]

commit 483d821108791092798f5d230686868112927044 upstream.

Unregister GPIOs requested through sysfs at chip remove to avoid leaking
the associated memory and sysfs entries.

The stale sysfs entries prevented the gpio numbers from being exported
when the gpio range was later reused (e.g. at device reconnect).

This also fixes the related module-reference leak.

Note that kernfs makes sure that any on-going sysfs operations finish
before the class devices are unregistered and that further accesses
fail.

The chip exported flag is used to prevent gpiod exports during removal.
This also makes it harder to trigger, but does not fix, the related race
between gpiochip_remove and export_store, which is really a race with
gpiod_request that needs to be addressed separately.

Also note that this would prevent the crashes (e.g. NULL-dereferences)
at reconnect that affects pre-3.18 kernels, as well as use-after-free on
operations on open attribute files on pre-3.14 kernels (prior to
kernfs).

Fixes: d8f388d8dc8d ("gpio: sysfs interface")
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 7fc186c6927d..a03e18f5e562 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -800,6 +800,7 @@ static struct class gpio_class = {
  */
 int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 {
+	struct gpio_chip	*chip;
 	unsigned long		flags;
 	int			status;
 	const char		*ioname = NULL;
@@ -817,8 +818,16 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 		return -EINVAL;
 	}
 
+	chip = desc->chip;
+
 	mutex_lock(&sysfs_lock);
 
+	/* check if chip is being removed */
+	if (!chip || !chip->exported) {
+		status = -ENODEV;
+		goto fail_unlock;
+	}
+
 	spin_lock_irqsave(&gpio_lock, flags);
 	if (!test_bit(FLAG_REQUESTED, &desc->flags) ||
 	     test_bit(FLAG_EXPORT, &desc->flags)) {
@@ -1057,12 +1066,15 @@ static void gpiochip_unexport(struct gpio_chip *chip)
 {
 	int			status;
 	struct device		*dev;
+	struct gpio_desc *desc;
+	unsigned int i;
 
 	mutex_lock(&sysfs_lock);
 	dev = class_find_device(&gpio_class, NULL, chip, match_export);
 	if (dev) {
 		put_device(dev);
 		device_unregister(dev);
+		/* prevent further gpiod exports */
 		chip->exported = false;
 		status = 0;
 	} else
@@ -1071,6 +1083,13 @@ static void gpiochip_unexport(struct gpio_chip *chip)
 
 	if (status)
 		chip_dbg(chip, "%s: status %d\n", __func__, status);
+
+	/* unregister gpiod class devices owned by sysfs */
+	for (i = 0; i < chip->ngpio; i++) {
+		desc = &chip->desc[i];
+		if (test_and_clear_bit(FLAG_SYSFS, &desc->flags))
+			gpiod_free(desc);
+	}
 }
 
 static int __init gpiolib_sysfs_init(void)
-- 
2.3.6

Re: Patch "gpio: sysfs: fix memory leaks and device hotplug" has been added to the 3.14-stable tree

[Greg KH]

On Fri, May 15, 2015 at 12:07:54PM +0200, Johan Hovold wrote:
> On Thu, May 14, 2015 at 06:57:31PM -0700, Greg Kroah-Hartman wrote:
> > 
> > This is a note to let you know that I've just added the patch titled
> > 
> >     gpio: sysfs: fix memory leaks and device hotplug
> > 
> > to the 3.14-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > 
> > The filename of the patch is:
> >      gpio-sysfs-fix-memory-leaks-and-device-hotplug.patch
> > and it can be found in the queue-3.14 subdirectory.
> > 
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> 
> Your backport looks good, but this one depends on commit 01cca93a9491
> ("gpio: unregister gpiochip device before removing it") which needs to
> be backported as well. I tried to convey this in the stable tag:
> 
> 	Cc: stable <stable@vger.kernel.org> # v2.6.27: 01cca93a9491

You did that correctly, I didn't read it properly, it's my fault here,
sorry about that.

Thanks for the patches, I'll go queue them up now.

greg k-h