[PATCH v2] gpio: gpiolib-of: Fix refcount bugs in of_mm_gpiochip_add_data()

[PATCH v2] gpio: gpiolib-of: Fix refcount bugs in of_mm_gpiochip_add_data()

[Liang He]

We should use of_node_get() when a new reference of device_node
is created. It is noted that the old reference stored in
'mm_gc->gc.of_node' should also be decreased.

This patch is based on the fact that there is a call site in function
'qe_add_gpiochips()' of src file 'drivers\soc\fsl\qe\gpio.c'. In this
function, of_mm_gpiochip_add_data() is contained in an iteration of
for_each_compatible_node() which will automatically increase and
decrease the refcount. So we need additional of_node_get() for the
reference escape in of_mm_gpiochip_add_data().

Fixes: a19e3da5bc5f ("of/gpio: Kill of_gpio_chip and add members directly to gpio_chip")
Signed-off-by: Liang He <windhl@126.com>
---
 changelog:

 v2: (1) add more explaination advised by Linus Walleij
     (2) use correct fix tag
 v1: fix the refcount bug by
 https://lore.kernel.org/all/20220704091313.277567-1-windhl@126.com/

 NOTE: I have confirmed that the mm_gc->gc.of_node has been correctly
put by
of_mm_gpiochip_remove()-->gpiochip_remove()-->of_gpiochip_remove()

 drivers/gpio/gpiolib-of.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 3d6c3ffd5576..de100b0217da 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -860,7 +860,8 @@ int of_mm_gpiochip_add_data(struct device_node *np,
 	if (mm_gc->save_regs)
 		mm_gc->save_regs(mm_gc);
 
-	mm_gc->gc.of_node = np;
+	of_node_put(mm_gc->gc.of_node);
+	mm_gc->gc.of_node = of_node_get(np);
 
 	ret = gpiochip_add_data(gc, data);
 	if (ret)
@@ -868,6 +869,7 @@ int of_mm_gpiochip_add_data(struct device_node *np,
 
 	return 0;
 err2:
+	of_node_put(np);
 	iounmap(mm_gc->regs);
 err1:
 	kfree(gc->label);
-- 
2.25.1

Re: [PATCH v2] gpio: gpiolib-of: Fix refcount bugs in of_mm_gpiochip_add_data()

[Linus Walleij]

On Mon, Jul 11, 2022 at 2:52 PM Liang He <windhl@126.com> wrote:

> We should use of_node_get() when a new reference of device_node
> is created. It is noted that the old reference stored in
> 'mm_gc->gc.of_node' should also be decreased.
>
> This patch is based on the fact that there is a call site in function
> 'qe_add_gpiochips()' of src file 'drivers\soc\fsl\qe\gpio.c'. In this
> function, of_mm_gpiochip_add_data() is contained in an iteration of
> for_each_compatible_node() which will automatically increase and
> decrease the refcount. So we need additional of_node_get() for the
> reference escape in of_mm_gpiochip_add_data().
>
> Fixes: a19e3da5bc5f ("of/gpio: Kill of_gpio_chip and add members directly to gpio_chip")
> Signed-off-by: Liang He <windhl@126.com>

Patch applied for next as nonurgent fix!

Yours,
Linus Walleij

Re: [PATCH v2] gpio: gpiolib-of: Fix refcount bugs in of_mm_gpiochip_add_data()

[Linus Walleij]

On Mon, Jul 11, 2022 at 3:23 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Jul 11, 2022 at 2:52 PM Liang He <windhl@126.com> wrote:
>
> > We should use of_node_get() when a new reference of device_node
> > is created. It is noted that the old reference stored in
> > 'mm_gc->gc.of_node' should also be decreased.
> >
> > This patch is based on the fact that there is a call site in function
> > 'qe_add_gpiochips()' of src file 'drivers\soc\fsl\qe\gpio.c'. In this
> > function, of_mm_gpiochip_add_data() is contained in an iteration of
> > for_each_compatible_node() which will automatically increase and
> > decrease the refcount. So we need additional of_node_get() for the
> > reference escape in of_mm_gpiochip_add_data().
> >
> > Fixes: a19e3da5bc5f ("of/gpio: Kill of_gpio_chip and add members directly to gpio_chip")
> > Signed-off-by: Liang He <windhl@126.com>
>
> Patch applied for next as nonurgent fix!

Oh no I shouldn't. This goes to the GPIO tree, took it out again.

Yours,
Linus Walleij

Re: [PATCH v2] gpio: gpiolib-of: Fix refcount bugs in of_mm_gpiochip_add_data()

[Bartosz Golaszewski]

On Tue, Jul 12, 2022 at 10:26 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Jul 11, 2022 at 3:23 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Mon, Jul 11, 2022 at 2:52 PM Liang He <windhl@126.com> wrote:
> >
> > > We should use of_node_get() when a new reference of device_node
> > > is created. It is noted that the old reference stored in
> > > 'mm_gc->gc.of_node' should also be decreased.
> > >
> > > This patch is based on the fact that there is a call site in function
> > > 'qe_add_gpiochips()' of src file 'drivers\soc\fsl\qe\gpio.c'. In this
> > > function, of_mm_gpiochip_add_data() is contained in an iteration of
> > > for_each_compatible_node() which will automatically increase and
> > > decrease the refcount. So we need additional of_node_get() for the
> > > reference escape in of_mm_gpiochip_add_data().
> > >
> > > Fixes: a19e3da5bc5f ("of/gpio: Kill of_gpio_chip and add members directly to gpio_chip")
> > > Signed-off-by: Liang He <windhl@126.com>
> >
> > Patch applied for next as nonurgent fix!
>
> Oh no I shouldn't. This goes to the GPIO tree, took it out again.
>
> Yours,
> Linus Walleij

Applied to GPIO tree.

Bart