linux-kernel - [PATCH] gpiolib: fix a few issues with gpiochip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1409930524-1529-1-git-send-email-octavian.purdila@intel.com>
Date:	Fri,  5 Sep 2014 18:22:04 +0300
From:	Octavian Purdila <octavian.purdila@...el.com>
To:	linus.walleij@...aro.org, gnurou@...il.com
Cc:	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
	Octavian Purdila <octavian.purdila@...el.com>
Subject: [PATCH] gpiolib: fix a few issues with gpiochip_remove

The current implementation of gpiochip_remove() does not check to see
if the GPIO pins are busy before removing the associated irqchip and
this is causing the following warning:

WARNING: CPU: 3 PID: 553 at fs/proc/generic.c:521 remove_proc_entry+0x19f/0x1b0()
remove_proc_entry: removing non-empty directory 'irq/24', leaking at least 'bmc150_accel_event'
Call Trace:
 [<ffffffff81a78504>] dump_stack+0x4e/0x7a
 [<ffffffff810c79bd>] warn_slowpath_common+0x7d/0xa0
 [<ffffffff810c7a2c>] warn_slowpath_fmt+0x4c/0x50
 [<ffffffff8125259f>] remove_proc_entry+0x19f/0x1b0
 [<ffffffff811138ae>] unregister_irq_proc+0xce/0xf0
 [<ffffffff8110dbc1>] free_desc+0x31/0x70
 [<ffffffff8110dc3c>] irq_free_descs+0x3c/0x80
 [<ffffffff81113096>] irq_dispose_mapping+0x36/0x50
 [<ffffffff8148549a>] gpiochip_remove+0x5a/0x160
 [<ffffffff814895d8>] dln2_do_remove+0x18/0x80
 [<ffffffff8148966a>] dln2_gpio_remove+0x2a/0x30
 [<ffffffff816143bd>] platform_drv_remove+0x1d/0x40
...

and bug:

BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97
in_atomic(): 1, irqs_disabled(): 1, pid: 553, name: khubd
Preemption disabled at:[<ffffffff81485462>] gpiochip_remove+0x22/0x160
Call Trace:
 [<ffffffff81a78504>] dump_stack+0x4e/0x7a
 [<ffffffff810e8dff>] __might_sleep+0x10f/0x180
 [<ffffffff81a7f3f0>] mutex_lock+0x20/0x3d
 [<ffffffff8110dbcd>] free_desc+0x3d/0x70
 [<ffffffff8110dc3c>] irq_free_descs+0x3c/0x80
 [<ffffffff81113096>] irq_dispose_mapping+0x36/0x50
 [<ffffffff8148549a>] gpiochip_remove+0x5a/0x160
 [<ffffffff814895d8>] dln2_do_remove+0x18/0x80
 [<ffffffff8148966a>] dln2_gpio_remove+0x2a/0x30
 [<ffffffff816143bd>] platform_drv_remove+0x1d/0x40
...

The current implementaion also does a partial cleanup if one of the
pins is busy, which makes it impossible to retry the remove operation
later.

A retry operation is needed in the case of MFD devices that bundles a
GPIO device and another device that is an indirect consumer of the
GPIO device (typical an I2C bus).

In this case, when the MFD device is removed, if an I2C device
associated with the I2C bus of the MFD device is using a GPIO pin (as
an interrupt source for example), and the remove routine for the GPIO
device is called first, then the removal of the gpio chip will fail.

However, we can later retry the gpio chip removal, as the I2C bus will
eventually be removed which will cause the I2C device to release the
GPIO pin.

This patch modifies gpiochip_remove to be atomic (i.e. if it fails no
partial cleanup is done) and it also moves gpiochip_irqchip_remove()
out of the spinlock to avoid the bug above.

Signed-off-by: Octavian Purdila <octavian.purdila@...el.com>
---
 drivers/gpio/gpiolib.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 15cc0bb..0f53bef 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -314,14 +314,8 @@ int gpiochip_remove(struct gpio_chip *chip)
 	int		status = 0;
 	unsigned	id;

-	acpi_gpiochip_remove(chip);
-
 	spin_lock_irqsave(&gpio_lock, flags);

-	gpiochip_irqchip_remove(chip);
-	gpiochip_remove_pin_ranges(chip);
-	of_gpiochip_remove(chip);
-
 	for (id = 0; id < chip->ngpio; id++) {
 		if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) {
 			status = -EBUSY;
@@ -337,8 +331,13 @@ int gpiochip_remove(struct gpio_chip *chip)

 	spin_unlock_irqrestore(&gpio_lock, flags);

-	if (status == 0)
+	if (status == 0) {
+		gpiochip_irqchip_remove(chip);
+		gpiochip_remove_pin_ranges(chip);
+		of_gpiochip_remove(chip);
+		acpi_gpiochip_remove(chip);
 		gpiochip_unexport(chip);
+	}

 	return status;
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/