gpio: fix gpio_chip add and remove
Subsystems (2)
cover gpio: fix gpio_chip add and remove
This series fix a few issues with gpiochip_add and gpiochip_remove.
The gpio-chip removal paths have probably not been exercised much, and there are further issues here that I'm working on fixing in order better support hot-plugging of gpio chips. I believe these patches should go into 3.19 meanwhile.
Johan
Johan Hovold (6):
gpio: fix memory and reference leaks in gpiochip_add error path
gpio: fix gpio-chip list corruption
gpio: clean up gpiochip_add error handling
gpio: fix memory leak and sleep-while-atomic
gpio: fix sleep-while-atomic in gpiochip_remove
gpio: unregister gpiochip device before removing it
drivers/gpio/gpiolib.c | 58 ++++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 28 deletions(-)
Signature · 1 line
2.0.5
PATCH 1/6 gpio: fix memory and reference leaks in gpiochip_add error path
Memory allocated and references taken by of_gpiochip_add and acpi_gpiochip_add were never released on errors in gpiochip_add (e.g. failure to find free gpio range).
has a node pointer")
automatically")
Cc: stable <[email protected]>
--- drivers/gpio/gpiolib.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/gpio/gpiolib.c +7 -4
@@ -277,6 +277,9 @@ int gpiochip_add(struct gpio_chip *chip)spin_unlock_irqrestore (&gpio_lock, flags); +if (status) + goto fail; +#ifdef CONFIG_PINCTRL INIT_LIST_HEAD (&chip->pin_ranges);#endif
@@ -284,12 +287,12 @@ int gpiochip_add(struct gpio_chip *chip)of_gpiochip_add (chip);acpi_gpiochip_add (chip); -if (status) - goto fail; - status =gpiochip_export (chip); -if (status) +if (status) { +acpi_gpiochip_remove (chip); +of_gpiochip_remove (chip); goto fail; + } pr_debug("%s: registered GPIOs %d to %d on device: %s\n", __func__, chip->base, chip->base + chip->ngpio - 1,
Signature · 1 line
2.0.5
PATCH 2/6 gpio: fix gpio-chip list corruption
In reply to Johan HovoldFix potential corruption of gpio-chip list due to failure to remove the chip from the list before returning in gpiochip_add error path.
The chip could be long gone when the global list is next traversed, something which could lead to a null-pointer dereference. In the best case (chip not deallocated) we are just leaking the gpio range.
--- drivers/gpio/gpiolib.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/gpio/gpiolib.c +10 -7
@@ -248,7 +248,8 @@ int gpiochip_add(struct gpio_chip *chip) base =gpiochip_find_base (chip->ngpio);if (base <0 ) { status = base; - goto unlock; +spin_unlock_irqrestore (&gpio_lock, flags); + goto err_free_descs; } chip->base = base; }
@@ -288,11 +289,8 @@ int gpiochip_add(struct gpio_chip *chip)acpi_gpiochip_add (chip); status =gpiochip_export (chip); -if (status) { -acpi_gpiochip_remove (chip); -of_gpiochip_remove (chip); - goto fail; - } +if (status) + goto err_remove_chip; pr_debug("%s: registered GPIOs %d to %d on device: %s\n", __func__, chip->base, chip->base + chip->ngpio - 1,
@@ -300,9 +298,14 @@ int gpiochip_add(struct gpio_chip *chip)return 0 ; -unlock: +err_remove_chip: +acpi_gpiochip_remove (chip); +of_gpiochip_remove (chip); +spin_lock_irqsave (&gpio_lock, flags); +list_del (&chip->list);spin_unlock_irqrestore (&gpio_lock, flags); fail: +err_free_descs:kfree (descs); chip->desc =NULL ;
Signature · 1 line
2.0.5
PATCH 3/6 gpio: clean up gpiochip_add error handling
In reply to Johan HovoldClean up gpiochip_add error handling.
--- drivers/gpio/gpiolib.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-)
drivers/gpio/gpiolib.c +17 -21
@@ -255,32 +255,29 @@ int gpiochip_add(struct gpio_chip *chip) } status =gpiochip_add_to_list (chip); +if (status) { +spin_unlock_irqrestore (&gpio_lock, flags); + goto err_free_descs; + } -if (status ==0 ) { -for (id =0 ; id < chip->ngpio; id++) { -struct gpio_desc *desc = &descs[id]; - desc->chip = chip; - -/* REVISIT: most hardware initializes GPIOs as -* inputs (often with pullups enabled) so power -* usage is minimized. Linux code should set the -* gpio direction first thing; but until it does, -* and in case chip->get_direction is not set, -* we may expose the wrong direction in sysfs. -*/ - desc->flags = !chip->direction_input - ? (1 <<FLAG_IS_OUT ) - :0 ; - } +for (id =0 ; id < chip->ngpio; id++) { +struct gpio_desc *desc = &descs[id]; + + desc->chip = chip; + +/* REVISIT: most hardware initializes GPIOs as inputs (often +* with pullups enabled) so power usage is minimized. Linux +* code should set the gpio direction first thing; but until +* it does, and in case chip->get_direction is not set, we may +* expose the wrong direction in sysfs. +*/ + desc->flags = !chip->direction_input ? (1 <<FLAG_IS_OUT ) :0 ; } chip->desc = descs;spin_unlock_irqrestore (&gpio_lock, flags); -if (status) - goto fail; -#ifdef CONFIG_PINCTRL INIT_LIST_HEAD (&chip->pin_ranges);#endif
@@ -304,10 +301,9 @@ err_remove_chip:spin_lock_irqsave (&gpio_lock, flags);list_del (&chip->list);spin_unlock_irqrestore (&gpio_lock, flags); -fail: + chip->desc =NULL ; err_free_descs:kfree (descs); - chip->desc =NULL ;/* failures here can mean systems won't boot... */ pr_err("%s: GPIOs %d..%d (%s) failed to register\n", __func__,
Signature · 1 line
2.0.5
PATCH 4/6 gpio: fix memory leak and sleep-while-atomic
In reply to Johan HovoldFix memory leak and sleep-while-atomic in gpiochip_remove.
The memory leak was introduced by afa82fab5e13 ("gpio / ACPI: Move event handling registration to gpiolib irqchip helpers") that moved the release of acpi interrupt resources to gpiochip_irqchip_remove, but by then the resources are no longer accessible as the acpi_gpio_chip has already been freed by acpi_gpiochip_remove.
Note that this also fixes a few potential sleep-while-atomics, which has been around since 1425052097b5 ("gpio: add IRQ chip helpers in gpiolib") when the call to gpiochip_irqchip_remove while holding a spinlock was added (a couple of irq-domain paths can end up grabbing mutexes).
gpiolib irqchip helpers")
--- drivers/gpio/gpiolib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/gpio/gpiolib.c +2 -1
@@ -327,11 +327,12 @@ void gpiochip_remove(struct gpio_chip *chip)unsigned long flags;unsigned id; +gpiochip_irqchip_remove (chip); +acpi_gpiochip_remove (chip);spin_lock_irqsave (&gpio_lock, flags); -gpiochip_irqchip_remove (chip);gpiochip_remove_pin_ranges (chip);of_gpiochip_remove (chip);
Signature · 1 line
2.0.5
PATCH 5/6 gpio: fix sleep-while-atomic in gpiochip_remove
In reply to Johan HovoldMove direct and indirect calls to gpiochip_remove_pin_ranges outside of spin lock as they can end up taking a mutex in pinctrl_remove_gpio_range.
Note that the pin ranges are already added outside of the lock.
--- drivers/gpio/gpiolib.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/gpio/gpiolib.c +1 -3
@@ -330,12 +330,10 @@ void gpiochip_remove(struct gpio_chip *chip)gpiochip_irqchip_remove (chip);acpi_gpiochip_remove (chip); - -spin_lock_irqsave (&gpio_lock, flags); -gpiochip_remove_pin_ranges (chip);of_gpiochip_remove (chip); +spin_lock_irqsave (&gpio_lock, flags);for (id =0 ; id < chip->ngpio; id++) {if (test_bit (FLAG_REQUESTED , &chip->desc[id].flags))dev_crit (chip->dev,"REMOVING GPIOCHIP WITH GPIOS STILL REQUESTED\n" );
Signature · 1 line
2.0.5
PATCH 6/6 gpio: unregister gpiochip device before removing it
In reply to Johan HovoldUnregister gpiochip device (used to export information through sysfs) before removing it internally. This way removal will reverse addition.
--- drivers/gpio/gpiolib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/gpio/gpiolib.c +2 -1
@@ -327,6 +327,8 @@ void gpiochip_remove(struct gpio_chip *chip)unsigned long flags;unsigned id; +gpiochip_unexport (chip); +gpiochip_irqchip_remove (chip);acpi_gpiochip_remove (chip);
@@ -343,7 +345,6 @@ void gpiochip_remove(struct gpio_chip *chip)list_del (&chip->list);spin_unlock_irqrestore (&gpio_lock, flags); -gpiochip_unexport (chip);kfree (chip->desc); chip->desc =NULL ;
Signature · 1 line
2.0.5
Quoted 6 lines · Johan Hovold
This series fix a few issues with
gpiochip_addandgpiochip_remove.The gpio-chip removal paths have probably not been exercised much, and there are further issues here that I'm working on fixing in order better support hot-plugging of gpio chips. I believe these patches should go into 3.19 meanwhile.
GOOD STUFF!
All six applied for fixes.
Yours,
Linus Walleij