[PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[Tzung-Bi Shih]

`kobj->name` should be freed by kfree_const()[1][2].  Correct it.

[1] https://elixir.bootlin.com/linux/v6.18/source/lib/kasprintf.c#L41
[2] https://elixir.bootlin.com/linux/v6.18/source/lib/kobject.c#L695

Cc: stable@vger.kernel.org
Fixes: c351bb64cbe6 ("gpiolib: free device name on error path to fix kmemleak")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5eb918da7ea2..ba9323432e3a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1263,7 +1263,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 err_free_descs:
 	kfree(gdev->descs);
 err_free_dev_name:
-	kfree(dev_name(&gdev->dev));
+	kfree_const(dev_name(&gdev->dev));
 err_free_ida:
 	ida_free(&gpio_ida, gdev->id);
 err_free_gdev:
-- 
2.52.0.457.g6b5491de43-goog

Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[Bartosz Golaszewski]

On Fri, 16 Jan 2026 09:10:14 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> `kobj->name` should be freed by kfree_const()[1][2].  Correct it.
>
> [1] https://elixir.bootlin.com/linux/v6.18/source/lib/kasprintf.c#L41
> [2] https://elixir.bootlin.com/linux/v6.18/source/lib/kobject.c#L695
>

Please don't add links third-party groks to git commit messages.

> Cc: stable@vger.kernel.org
> Fixes: c351bb64cbe6 ("gpiolib: free device name on error path to fix kmemleak")
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
>  drivers/gpio/gpiolib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 5eb918da7ea2..ba9323432e3a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1263,7 +1263,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>  err_free_descs:
>  	kfree(gdev->descs);
>  err_free_dev_name:
> -	kfree(dev_name(&gdev->dev));
> +	kfree_const(dev_name(&gdev->dev));
>  err_free_ida:
>  	ida_free(&gpio_ida, gdev->id);
>  err_free_gdev:
> --
> 2.52.0.457.g6b5491de43-goog
>
>

I've never paid attention to this bit but it really looks broken. I understand
that this string won't get freed until we initialize refcounting on the
underlying kobject but reaching two abstraction layers below to get the string
for freeing out of the kobject looks incorrect to me.

It's also one of only two instances of doing kfree(dev_name(dev)), the other
one being in drivers/scsi/hosts.c.

It looks to me that the device name is not really used in
gpiochip_add_data_with_key(). Can we move dev_set_name() after
device_initialize()?

Bart

Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[Greg Kroah-Hartman]

On Fri, Jan 16, 2026 at 01:15:17PM +0000, Bartosz Golaszewski wrote:
> On Fri, 16 Jan 2026 09:10:14 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > `kobj->name` should be freed by kfree_const()[1][2].  Correct it.
> >
> > [1] https://elixir.bootlin.com/linux/v6.18/source/lib/kasprintf.c#L41
> > [2] https://elixir.bootlin.com/linux/v6.18/source/lib/kobject.c#L695
> >
> 
> Please don't add links third-party groks to git commit messages.
> 
> > Cc: stable@vger.kernel.org
> > Fixes: c351bb64cbe6 ("gpiolib: free device name on error path to fix kmemleak")
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > ---
> >  drivers/gpio/gpiolib.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index 5eb918da7ea2..ba9323432e3a 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -1263,7 +1263,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> >  err_free_descs:
> >  	kfree(gdev->descs);
> >  err_free_dev_name:
> > -	kfree(dev_name(&gdev->dev));
> > +	kfree_const(dev_name(&gdev->dev));
> >  err_free_ida:
> >  	ida_free(&gpio_ida, gdev->id);
> >  err_free_gdev:
> > --
> > 2.52.0.457.g6b5491de43-goog
> >
> >
> 
> I've never paid attention to this bit but it really looks broken. I understand
> that this string won't get freed until we initialize refcounting on the
> underlying kobject but reaching two abstraction layers below to get the string
> for freeing out of the kobject looks incorrect to me.
> 
> It's also one of only two instances of doing kfree(dev_name(dev)), the other
> one being in drivers/scsi/hosts.c.

That one is wrong, I already rejected it :)

> It looks to me that the device name is not really used in
> gpiochip_add_data_with_key(). Can we move dev_set_name() after
> device_initialize()?

This should be cleaned up automatically by the driver core, no need to
free this on its own.

thanks,

greg k-h

Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[Bartosz Golaszewski]

On Fri, Jan 16, 2026 at 2:27 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jan 16, 2026 at 01:15:17PM +0000, Bartosz Golaszewski wrote:
> > On Fri, 16 Jan 2026 09:10:14 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > > `kobj->name` should be freed by kfree_const()[1][2].  Correct it.
> > >
> > > [1] https://elixir.bootlin.com/linux/v6.18/source/lib/kasprintf.c#L41
> > > [2] https://elixir.bootlin.com/linux/v6.18/source/lib/kobject.c#L695
> > >
> >
> > Please don't add links third-party groks to git commit messages.
> >
> > > Cc: stable@vger.kernel.org
> > > Fixes: c351bb64cbe6 ("gpiolib: free device name on error path to fix kmemleak")
> > > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > > ---
> > >  drivers/gpio/gpiolib.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > index 5eb918da7ea2..ba9323432e3a 100644
> > > --- a/drivers/gpio/gpiolib.c
> > > +++ b/drivers/gpio/gpiolib.c
> > > @@ -1263,7 +1263,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> > >  err_free_descs:
> > >     kfree(gdev->descs);
> > >  err_free_dev_name:
> > > -   kfree(dev_name(&gdev->dev));
> > > +   kfree_const(dev_name(&gdev->dev));
> > >  err_free_ida:
> > >     ida_free(&gpio_ida, gdev->id);
> > >  err_free_gdev:
> > > --
> > > 2.52.0.457.g6b5491de43-goog
> > >
> > >
> >
> > I've never paid attention to this bit but it really looks broken. I understand
> > that this string won't get freed until we initialize refcounting on the
> > underlying kobject but reaching two abstraction layers below to get the string
> > for freeing out of the kobject looks incorrect to me.
> >
> > It's also one of only two instances of doing kfree(dev_name(dev)), the other
> > one being in drivers/scsi/hosts.c.
>
> That one is wrong, I already rejected it :)
>
> > It looks to me that the device name is not really used in
> > gpiochip_add_data_with_key(). Can we move dev_set_name() after
> > device_initialize()?
>
> This should be cleaned up automatically by the driver core, no need to
> free this on its own.
>

It will once we initiate kobject reference counting from
device_initialize(). The code here still hasn't called it though. It
doesn't look like it'll be freed to me on errors before
device_initialize(). A later patch in this series seems to try to
address it though so maybe this one's not even needed except for
backporting to stable branches.

Bartosz

Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[Tzung-Bi Shih]

On Fri, Jan 16, 2026 at 01:15:17PM +0000, Bartosz Golaszewski wrote:
> On Fri, 16 Jan 2026 09:10:14 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > `kobj->name` should be freed by kfree_const()[1][2].  Correct it.
> >
> > [1] https://elixir.bootlin.com/linux/v6.18/source/lib/kasprintf.c#L41
> > [2] https://elixir.bootlin.com/linux/v6.18/source/lib/kobject.c#L695
> >
> 
> Please don't add links third-party groks to git commit messages.
> 
> > Cc: stable@vger.kernel.org
> > Fixes: c351bb64cbe6 ("gpiolib: free device name on error path to fix kmemleak")
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > ---
> >  drivers/gpio/gpiolib.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index 5eb918da7ea2..ba9323432e3a 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -1263,7 +1263,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> >  err_free_descs:
> >  	kfree(gdev->descs);
> >  err_free_dev_name:
> > -	kfree(dev_name(&gdev->dev));
> > +	kfree_const(dev_name(&gdev->dev));
> >  err_free_ida:
> >  	ida_free(&gpio_ida, gdev->id);
> >  err_free_gdev:
> > --
> > 2.52.0.457.g6b5491de43-goog
> >
> >
> 
> I've never paid attention to this bit but it really looks broken. I understand
> that this string won't get freed until we initialize refcounting on the
> underlying kobject but reaching two abstraction layers below to get the string
> for freeing out of the kobject looks incorrect to me.
> 
> It's also one of only two instances of doing kfree(dev_name(dev)), the other
> one being in drivers/scsi/hosts.c.
> 
> It looks to me that the device name is not really used in

The device name is indeed used in gpiochip_add_data_with_key().  E.g.,
gpiochip_get_ngpios() -> dev_err().

> gpiochip_add_data_with_key(). Can we move dev_set_name() after
> device_initialize()?

Sounds a good idea.  I'll try to approach it in accompany with the
aggressive patch 03/23.

Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[Jason Gunthorpe]

On Fri, Jan 16, 2026 at 08:10:14AM +0000, Tzung-Bi Shih wrote:
> `kobj->name` should be freed by kfree_const()[1][2].  Correct it.
> 
> [1] https://elixir.bootlin.com/linux/v6.18/source/lib/kasprintf.c#L41
> [2] https://elixir.bootlin.com/linux/v6.18/source/lib/kobject.c#L695
> 
> Cc: stable@vger.kernel.org
> Fixes: c351bb64cbe6 ("gpiolib: free device name on error path to fix kmemleak")
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
>  drivers/gpio/gpiolib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 5eb918da7ea2..ba9323432e3a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1263,7 +1263,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>  err_free_descs:
>  	kfree(gdev->descs);
>  err_free_dev_name:
> -	kfree(dev_name(&gdev->dev));
> +	kfree_const(dev_name(&gdev->dev));
>  err_free_ida:
>  	ida_free(&gpio_ida, gdev->id);
>  err_free_gdev:
        kfree(gdev);

I don't think users should be open coding this, put_device() frees the
dev_name properly. The issue here is that the code doesn't call
device_initialize() before doing dev_set_name() and then tries to
fiddle a weird teardown sequence when it eventually does get initialized:

err_remove_from_list:
        if (gdev->dev.release) {
                /* release() has been registered by gpiochip_setup_dev() */
                gpio_device_put(gdev);
                goto err_print_message;
        }

If gpiochip_add_data_with_key() is split into two functions, one that
does kzalloc(), some initialization and then ends with
device_initialize(), then a second function that calls the first and
does the rest of the initialization and error unwinds with
put_device() it will work a lot better.

Jason

Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[Bartosz Golaszewski]

On Fri, Jan 16, 2026 at 3:14 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Fri, Jan 16, 2026 at 08:10:14AM +0000, Tzung-Bi Shih wrote:
> > `kobj->name` should be freed by kfree_const()[1][2].  Correct it.
> >
> > [1] https://elixir.bootlin.com/linux/v6.18/source/lib/kasprintf.c#L41
> > [2] https://elixir.bootlin.com/linux/v6.18/source/lib/kobject.c#L695
> >
> > Cc: stable@vger.kernel.org
> > Fixes: c351bb64cbe6 ("gpiolib: free device name on error path to fix kmemleak")
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > ---
> >  drivers/gpio/gpiolib.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index 5eb918da7ea2..ba9323432e3a 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -1263,7 +1263,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> >  err_free_descs:
> >       kfree(gdev->descs);
> >  err_free_dev_name:
> > -     kfree(dev_name(&gdev->dev));
> > +     kfree_const(dev_name(&gdev->dev));
> >  err_free_ida:
> >       ida_free(&gpio_ida, gdev->id);
> >  err_free_gdev:
>         kfree(gdev);
>
> I don't think users should be open coding this, put_device() frees the
> dev_name properly. The issue here is that the code doesn't call
> device_initialize() before doing dev_set_name() and then tries to
> fiddle a weird teardown sequence when it eventually does get initialized:
>
> err_remove_from_list:
>         if (gdev->dev.release) {
>                 /* release() has been registered by gpiochip_setup_dev() */
>                 gpio_device_put(gdev);
>                 goto err_print_message;
>         }
>
> If gpiochip_add_data_with_key() is split into two functions, one that
> does kzalloc(), some initialization and then ends with
> device_initialize(), then a second function that calls the first and
> does the rest of the initialization and error unwinds with
> put_device() it will work a lot better.
>

In theory yes but you wouldn't be the first one to attempt to improve
it. This code is very brittle when it comes to GPIO chips that need to
be initialized very early into the boot process. I'm talking old
drivers in arch which call this function without even an associated
parent struct device. When I'm looking at it now, it does seem
possible to call device_initialize() early but whether that will work
correctly for all existing users is a bigger question.

I'm open to trying it after v7.0-rc1 is tagged. This would give it
enough time in linux-next to make sure it works.

Bartosz

Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[Tzung-Bi Shih]

On Fri, Jan 16, 2026 at 03:38:37PM +0100, Bartosz Golaszewski wrote:
> On Fri, Jan 16, 2026 at 3:14 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Fri, Jan 16, 2026 at 08:10:14AM +0000, Tzung-Bi Shih wrote:
> > > `kobj->name` should be freed by kfree_const()[1][2].  Correct it.
> > >
> > > [1] https://elixir.bootlin.com/linux/v6.18/source/lib/kasprintf.c#L41
> > > [2] https://elixir.bootlin.com/linux/v6.18/source/lib/kobject.c#L695
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: c351bb64cbe6 ("gpiolib: free device name on error path to fix kmemleak")
> > > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > > ---
> > >  drivers/gpio/gpiolib.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > index 5eb918da7ea2..ba9323432e3a 100644
> > > --- a/drivers/gpio/gpiolib.c
> > > +++ b/drivers/gpio/gpiolib.c
> > > @@ -1263,7 +1263,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> > >  err_free_descs:
> > >       kfree(gdev->descs);
> > >  err_free_dev_name:
> > > -     kfree(dev_name(&gdev->dev));
> > > +     kfree_const(dev_name(&gdev->dev));
> > >  err_free_ida:
> > >       ida_free(&gpio_ida, gdev->id);
> > >  err_free_gdev:
> >         kfree(gdev);
> >
> > I don't think users should be open coding this, put_device() frees the
> > dev_name properly. The issue here is that the code doesn't call
> > device_initialize() before doing dev_set_name() and then tries to
> > fiddle a weird teardown sequence when it eventually does get initialized:
> >
> > err_remove_from_list:
> >         if (gdev->dev.release) {
> >                 /* release() has been registered by gpiochip_setup_dev() */
> >                 gpio_device_put(gdev);
> >                 goto err_print_message;
> >         }
> >
> > If gpiochip_add_data_with_key() is split into two functions, one that
> > does kzalloc(), some initialization and then ends with
> > device_initialize(), then a second function that calls the first and
> > does the rest of the initialization and error unwinds with
> > put_device() it will work a lot better.

That's basically what the aggressive patch 03/23 tries to do without
separating the first half to an indepedent function.

Generally, I think we can try to move device_initialize() earlier in the
function.  On error handling paths, just put_device() for it.  In the
.release() callback, free the resource iff it has initialized.

> In theory yes but you wouldn't be the first one to attempt to improve
> it. This code is very brittle when it comes to GPIO chips that need to
> be initialized very early into the boot process. I'm talking old
> drivers in arch which call this function without even an associated
> parent struct device. When I'm looking at it now, it does seem
> possible to call device_initialize() early but whether that will work
> correctly for all existing users is a bigger question.

FWIW: found a very early stage calling path when I was investigating
`gpiolib_initialized`: start_kernel() -> init_IRQ() -> dove_init_irq() ->
orion_gpio_init() -> gpiochip_add_data() -> gpiochip_add_data_with_key().

Prior to aab5c6f20023 ("gpio: set device type for GPIO chips"),
device_initialize() is also called in gpiochip_add_data_with_key().  It
seems to me it's possible to move it back to gpiochip_add_data_with_key()
as 03/23 does, and move it earlier in the function.

Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

[Bartosz Golaszewski]

On Tue, Jan 20, 2026 at 5:30 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> Generally, I think we can try to move device_initialize() earlier in the
> function.  On error handling paths, just put_device() for it.  In the
> .release() callback, free the resource iff it has initialized.
>
> > In theory yes but you wouldn't be the first one to attempt to improve
> > it. This code is very brittle when it comes to GPIO chips that need to
> > be initialized very early into the boot process. I'm talking old
> > drivers in arch which call this function without even an associated
> > parent struct device. When I'm looking at it now, it does seem
> > possible to call device_initialize() early but whether that will work
> > correctly for all existing users is a bigger question.
>
> FWIW: found a very early stage calling path when I was investigating
> `gpiolib_initialized`: start_kernel() -> init_IRQ() -> dove_init_irq() ->
> orion_gpio_init() -> gpiochip_add_data() -> gpiochip_add_data_with_key().
>
> Prior to aab5c6f20023 ("gpio: set device type for GPIO chips"),
> device_initialize() is also called in gpiochip_add_data_with_key().  It
> seems to me it's possible to move it back to gpiochip_add_data_with_key()
> as 03/23 does, and move it earlier in the function.

Sounds good, let's try it next cycle!

Tzung-Bi: please make it a change separate from the wider Revocable
series for GPIO.

Bart

[PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()

[Tzung-Bi Shih]

On error handling paths, gpiolib_cdev_register() doesn't free the
allocated resources which results leaks.  Fix it.

Cc: stable@vger.kernel.org
Fixes: 7b9b77a8bba9 ("gpiolib: add a per-gpio_device line state notification workqueue")
Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/gpio/gpiolib-cdev.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 3735c9fe1502..ba1eae15852d 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2797,16 +2797,23 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
 
 	ret = cdev_device_add(&gdev->chrdev, &gdev->dev);
 	if (ret)
-		return ret;
+		goto err_free_workqueue;
 
 	guard(srcu)(&gdev->srcu);
 	gc = srcu_dereference(gdev->chip, &gdev->srcu);
-	if (!gc)
-		return -ENODEV;
+	if (!gc) {
+		ret = -ENODEV;
+		goto err_free_cdev;
+	}
 
 	gpiochip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);
 
 	return 0;
+err_free_cdev:
+	cdev_device_del(&gdev->chrdev, &gdev->dev);
+err_free_workqueue:
+	destroy_workqueue(gdev->line_state_wq);
+	return ret;
 }
 
 void gpiolib_cdev_unregister(struct gpio_device *gdev)
-- 
2.52.0.457.g6b5491de43-goog

Re: [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()

[Bartosz Golaszewski]

On Fri, Jan 16, 2026 at 9:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On error handling paths, gpiolib_cdev_register() doesn't free the
> allocated resources which results leaks.  Fix it.
>
> Cc: stable@vger.kernel.org
> Fixes: 7b9b77a8bba9 ("gpiolib: add a per-gpio_device line state notification workqueue")
> Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
>  drivers/gpio/gpiolib-cdev.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index 3735c9fe1502..ba1eae15852d 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -2797,16 +2797,23 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
>
>         ret = cdev_device_add(&gdev->chrdev, &gdev->dev);
>         if (ret)
> -               return ret;
> +               goto err_free_workqueue;
>

I need to drop this because it jumps over the guard(). I think you'll
have to free the workqueue locally here instead.

Can you send a separate v2?

Bart

>         guard(srcu)(&gdev->srcu);
>         gc = srcu_dereference(gdev->chip, &gdev->srcu);
> -       if (!gc)
> -               return -ENODEV;
> +       if (!gc) {
> +               ret = -ENODEV;
> +               goto err_free_cdev;
> +       }
>
>         gpiochip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);
>
>         return 0;
> +err_free_cdev:
> +       cdev_device_del(&gdev->chrdev, &gdev->dev);
> +err_free_workqueue:
> +       destroy_workqueue(gdev->line_state_wq);
> +       return ret;
>  }
>
>  void gpiolib_cdev_unregister(struct gpio_device *gdev)
> --
> 2.52.0.457.g6b5491de43-goog
>

Re: [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()

[Tzung-Bi Shih]

On Tue, Jan 20, 2026 at 09:50:42AM +0100, Bartosz Golaszewski wrote:
> On Fri, Jan 16, 2026 at 9:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > On error handling paths, gpiolib_cdev_register() doesn't free the
> > allocated resources which results leaks.  Fix it.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 7b9b77a8bba9 ("gpiolib: add a per-gpio_device line state notification workqueue")
> > Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > ---
> >  drivers/gpio/gpiolib-cdev.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> > index 3735c9fe1502..ba1eae15852d 100644
> > --- a/drivers/gpio/gpiolib-cdev.c
> > +++ b/drivers/gpio/gpiolib-cdev.c
> > @@ -2797,16 +2797,23 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
> >
> >         ret = cdev_device_add(&gdev->chrdev, &gdev->dev);
> >         if (ret)
> > -               return ret;
> > +               goto err_free_workqueue;
> >
> 
> I need to drop this because it jumps over the guard(). I think you'll
> have to free the workqueue locally here instead.
> 
> Can you send a separate v2?

v2: https://lore.kernel.org/linux-gpio/20260120092650.2305319-1-tzungbi@kernel.org/

Heads up: I'll respin the whole series for targeting v7.0-rc1 for:
- Rebase after you applied some of the patches.
- I found you prefer "gpio" to "gpiolib" in the title prefix.
- I found yet another build warning when testing with
  https://lore.kernel.org/linux-gpio/202601200022.ZFwz8K6u-lkp@intel.com/

Re: [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()

[Bartosz Golaszewski]

On Tue, Jan 20, 2026 at 10:34 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Tue, Jan 20, 2026 at 09:50:42AM +0100, Bartosz Golaszewski wrote:
> > On Fri, Jan 16, 2026 at 9:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > >
> > > On error handling paths, gpiolib_cdev_register() doesn't free the
> > > allocated resources which results leaks.  Fix it.
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: 7b9b77a8bba9 ("gpiolib: add a per-gpio_device line state notification workqueue")
> > > Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
> > > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > > ---
> > >  drivers/gpio/gpiolib-cdev.c | 13 ++++++++++---
> > >  1 file changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> > > index 3735c9fe1502..ba1eae15852d 100644
> > > --- a/drivers/gpio/gpiolib-cdev.c
> > > +++ b/drivers/gpio/gpiolib-cdev.c
> > > @@ -2797,16 +2797,23 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
> > >
> > >         ret = cdev_device_add(&gdev->chrdev, &gdev->dev);
> > >         if (ret)
> > > -               return ret;
> > > +               goto err_free_workqueue;
> > >
> >
> > I need to drop this because it jumps over the guard(). I think you'll
> > have to free the workqueue locally here instead.
> >
> > Can you send a separate v2?
>
> v2: https://lore.kernel.org/linux-gpio/20260120092650.2305319-1-tzungbi@kernel.org/
>
> Heads up: I'll respin the whole series for targeting v7.0-rc1 for:
> - Rebase after you applied some of the patches.

I guess this concerns the device_initialize() rework? Yeah v7.0-rc1 is
good timing.

> - I found you prefer "gpio" to "gpiolib" in the title prefix.

Just makes the line shorter.

> - I found yet another build warning when testing with
>   https://lore.kernel.org/linux-gpio/202601200022.ZFwz8K6u-lkp@intel.com/

Yeah, this is the one I referred to in my previous email, just forgot
to link it.

Bart

[PATCH 03/23] gpiolib: Fix resource leaks on errors in gpiochip_add_data_with_key()

[Tzung-Bi Shih]

Since commit aab5c6f20023 ("gpio: set device type for GPIO chips"),
`gdev->dev.release` is unset.  As a result, the reference count to
`gdev->dev` isn't dropped on the error handling paths.

Drop the reference on errors.

Also reorder the instructions to make the intent more clear.  Now
gpiochip_add_data_with_key() roughly looks like:

   >>> Some memory allocation.  Go to ERR ZONE 1 on errors.
   >>> device_initialize().

   (gpiodev_release() takes over the responsibility for freeing the
    resources of `gdev->dev`.  The following error handling paths
    shouldn't go through ERR ZONE 1 again which leads to double free.)

   >>> Some initialization mainly on `gdev`.
   >>> The rest of initialization.  Go to ERR ZONE 2 on errors.
   >>> Chip registration success and exit.

   >>> ERR ZONE 2.  gpio_device_put() and exit.
   >>> ERR ZONE 1.

Cc: stable@vger.kernel.org
Fixes: aab5c6f20023 ("gpio: set device type for GPIO chips")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/gpio/gpiolib.c | 81 ++++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 43 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index ba9323432e3a..6fac59716405 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -893,13 +893,15 @@ static const struct device_type gpio_dev_type = {
 #define gcdev_unregister(gdev)		device_del(&(gdev)->dev)
 #endif
 
+/*
+ * An initial reference count has been held in gpiochip_add_data_with_key().
+ * The caller should drop the reference via gpio_device_put() on errors.
+ */
 static int gpiochip_setup_dev(struct gpio_device *gdev)
 {
 	struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
 	int ret;
 
-	device_initialize(&gdev->dev);
-
 	/*
 	 * If fwnode doesn't belong to another device, it's safe to clear its
 	 * initialized flag.
@@ -965,9 +967,11 @@ static void gpiochip_setup_devs(void)
 	list_for_each_entry_srcu(gdev, &gpio_devices, list,
 				 srcu_read_lock_held(&gpio_devices_srcu)) {
 		ret = gpiochip_setup_dev(gdev);
-		if (ret)
+		if (ret) {
+			gpio_device_put(gdev);
 			dev_err(&gdev->dev,
 				"Failed to initialize gpio device (%d)\n", ret);
+		}
 	}
 }
 
@@ -1048,24 +1052,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	int base = 0;
 	int ret;
 
-	/*
-	 * First: allocate and populate the internal stat container, and
-	 * set up the struct device.
-	 */
 	gdev = kzalloc(sizeof(*gdev), GFP_KERNEL);
 	if (!gdev)
 		return -ENOMEM;
-
-	gdev->dev.type = &gpio_dev_type;
-	gdev->dev.bus = &gpio_bus_type;
-	gdev->dev.parent = gc->parent;
-	rcu_assign_pointer(gdev->chip, gc);
-
 	gc->gpiodev = gdev;
 	gpiochip_set_data(gc, data);
 
-	device_set_node(&gdev->dev, gpiochip_choose_fwnode(gc));
-
 	ret = ida_alloc(&gpio_ida, GFP_KERNEL);
 	if (ret < 0)
 		goto err_free_gdev;
@@ -1075,17 +1067,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	if (ret)
 		goto err_free_ida;
 
-	if (gc->parent && gc->parent->driver)
-		gdev->owner = gc->parent->driver->owner;
-	else if (gc->owner)
-		/* TODO: remove chip->owner */
-		gdev->owner = gc->owner;
-	else
-		gdev->owner = THIS_MODULE;
-
 	ret = gpiochip_get_ngpios(gc, &gdev->dev);
 	if (ret)
 		goto err_free_dev_name;
+	gdev->ngpio = gc->ngpio;
 
 	gdev->descs = kcalloc(gc->ngpio, sizeof(*gdev->descs), GFP_KERNEL);
 	if (!gdev->descs) {
@@ -1099,21 +1084,37 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 		goto err_free_descs;
 	}
 
-	gdev->ngpio = gc->ngpio;
-	gdev->can_sleep = gc->can_sleep;
-
-	rwlock_init(&gdev->line_state_lock);
-	RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
-	BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
-
 	ret = init_srcu_struct(&gdev->srcu);
 	if (ret)
 		goto err_free_label;
+	rcu_assign_pointer(gdev->chip, gc);
 
 	ret = init_srcu_struct(&gdev->desc_srcu);
 	if (ret)
 		goto err_cleanup_gdev_srcu;
 
+	device_initialize(&gdev->dev);
+	/* From this point, the .release() function cleans up gdev->dev */
+	gdev->dev.type = &gpio_dev_type;
+	gdev->dev.bus = &gpio_bus_type;
+	gdev->dev.parent = gc->parent;
+	device_set_node(&gdev->dev, gpiochip_choose_fwnode(gc));
+
+	gdev->can_sleep = gc->can_sleep;
+	rwlock_init(&gdev->line_state_lock);
+	RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
+	BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
+#ifdef CONFIG_PINCTRL
+	INIT_LIST_HEAD(&gdev->pin_ranges);
+#endif
+	if (gc->parent && gc->parent->driver)
+		gdev->owner = gc->parent->driver->owner;
+	else if (gc->owner)
+		/* TODO: remove chip->owner */
+		gdev->owner = gc->owner;
+	else
+		gdev->owner = THIS_MODULE;
+
 	scoped_guard(mutex, &gpio_devices_lock) {
 		/*
 		 * TODO: this allocates a Linux GPIO number base in the global
@@ -1128,7 +1129,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 			if (base < 0) {
 				ret = base;
 				base = 0;
-				goto err_cleanup_desc_srcu;
+				goto err_put_device;
 			}
 
 			/*
@@ -1148,14 +1149,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 		ret = gpiodev_add_to_list_unlocked(gdev);
 		if (ret) {
 			gpiochip_err(gc, "GPIO integer space overlap, cannot add chip\n");
-			goto err_cleanup_desc_srcu;
+			goto err_put_device;
 		}
 	}
 
-#ifdef CONFIG_PINCTRL
-	INIT_LIST_HEAD(&gdev->pin_ranges);
-#endif
-
 	if (gc->names)
 		gpiochip_set_desc_names(gc);
 
@@ -1249,13 +1246,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	scoped_guard(mutex, &gpio_devices_lock)
 		list_del_rcu(&gdev->list);
 	synchronize_srcu(&gpio_devices_srcu);
-	if (gdev->dev.release) {
-		/* release() has been registered by gpiochip_setup_dev() */
-		gpio_device_put(gdev);
-		goto err_print_message;
-	}
-err_cleanup_desc_srcu:
-	cleanup_srcu_struct(&gdev->desc_srcu);
+err_put_device:
+	gpio_device_put(gdev);
+	goto err_print_message;
+
 err_cleanup_gdev_srcu:
 	cleanup_srcu_struct(&gdev->srcu);
 err_free_label:
@@ -1268,6 +1262,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	ida_free(&gpio_ida, gdev->id);
 err_free_gdev:
 	kfree(gdev);
+
 err_print_message:
 	/* failures here can mean systems won't boot... */
 	if (ret != -EPROBE_DEFER) {
-- 
2.52.0.457.g6b5491de43-goog

[PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()

[Tzung-Bi Shih]

On error handling paths, lineinfo_changed_notify() doesn't free the
allocated resources which results leaks.  Fix it.

Cc: stable@vger.kernel.org
Fixes: d4cd0902c156 ("gpio: cdev: make sure the cdev fd is still active before emitting events")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/gpio/gpiolib-cdev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index ba1eae15852d..6196aab5ed74 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2549,7 +2549,7 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
 	ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
 	if (!ctx) {
 		pr_err("Failed to allocate memory for line info notification\n");
-		return NOTIFY_DONE;
+		goto err_put_fp;
 	}
 
 	ctx->chg.event_type = action;
@@ -2563,6 +2563,9 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
 	queue_work(ctx->gdev->line_state_wq, &ctx->work);
 
 	return NOTIFY_OK;
+err_put_fp:
+	fput(fp);
+	return NOTIFY_DONE;
 }
 
 static int gpio_device_unregistered_notify(struct notifier_block *nb,
-- 
2.52.0.457.g6b5491de43-goog

Re: [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()

[Bartosz Golaszewski]

On Fri, 16 Jan 2026 09:10:17 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> On error handling paths, lineinfo_changed_notify() doesn't free the
> allocated resources which results leaks.  Fix it.
>
> Cc: stable@vger.kernel.org
> Fixes: d4cd0902c156 ("gpio: cdev: make sure the cdev fd is still active before emitting events")
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
>  drivers/gpio/gpiolib-cdev.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index ba1eae15852d..6196aab5ed74 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -2549,7 +2549,7 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
>  	ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
>  	if (!ctx) {
>  		pr_err("Failed to allocate memory for line info notification\n");
> -		return NOTIFY_DONE;
> +		goto err_put_fp;
>  	}
>
>  	ctx->chg.event_type = action;
> @@ -2563,6 +2563,9 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
>  	queue_work(ctx->gdev->line_state_wq, &ctx->work);
>
>  	return NOTIFY_OK;
> +err_put_fp:
> +	fput(fp);
> +	return NOTIFY_DONE;
>  }
>
>  static int gpio_device_unregistered_notify(struct notifier_block *nb,
> --
> 2.52.0.457.g6b5491de43-goog
>
>

There's only one place where you need this fput(), please do it directly in
the error path of kzalloc() and drop the label.

Bart

Re: [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()

[Tzung-Bi Shih]

On Fri, Jan 16, 2026 at 01:26:01PM +0000, Bartosz Golaszewski wrote:
> On Fri, 16 Jan 2026 09:10:17 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > On error handling paths, lineinfo_changed_notify() doesn't free the
> > allocated resources which results leaks.  Fix it.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: d4cd0902c156 ("gpio: cdev: make sure the cdev fd is still active before emitting events")
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > ---
> >  drivers/gpio/gpiolib-cdev.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> > index ba1eae15852d..6196aab5ed74 100644
> > --- a/drivers/gpio/gpiolib-cdev.c
> > +++ b/drivers/gpio/gpiolib-cdev.c
> > @@ -2549,7 +2549,7 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
> >  	ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
> >  	if (!ctx) {
> >  		pr_err("Failed to allocate memory for line info notification\n");
> > -		return NOTIFY_DONE;
> > +		goto err_put_fp;
> >  	}
> >
> >  	ctx->chg.event_type = action;
> > @@ -2563,6 +2563,9 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
> >  	queue_work(ctx->gdev->line_state_wq, &ctx->work);
> >
> >  	return NOTIFY_OK;
> > +err_put_fp:
> > +	fput(fp);
> > +	return NOTIFY_DONE;
> >  }
> >
> >  static int gpio_device_unregistered_notify(struct notifier_block *nb,
> > --
> > 2.52.0.457.g6b5491de43-goog
> >
> >
> 
> There's only one place where you need this fput(), please do it directly in
> the error path of kzalloc() and drop the label.

Sure, just wanted to give a heads up: 17/23 might introduce the label back.

v2: https://lore.kernel.org/all/20260120030857.2144847-1-tzungbi@kernel.org

Re: [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()

[Bartosz Golaszewski]

On Tue, Jan 20, 2026 at 4:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Fri, Jan 16, 2026 at 01:26:01PM +0000, Bartosz Golaszewski wrote:
> > On Fri, 16 Jan 2026 09:10:17 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > > On error handling paths, lineinfo_changed_notify() doesn't free the
> > > allocated resources which results leaks.  Fix it.
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: d4cd0902c156 ("gpio: cdev: make sure the cdev fd is still active before emitting events")
> > > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > > ---
> > >  drivers/gpio/gpiolib-cdev.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> > > index ba1eae15852d..6196aab5ed74 100644
> > > --- a/drivers/gpio/gpiolib-cdev.c
> > > +++ b/drivers/gpio/gpiolib-cdev.c
> > > @@ -2549,7 +2549,7 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
> > >     ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
> > >     if (!ctx) {
> > >             pr_err("Failed to allocate memory for line info notification\n");
> > > -           return NOTIFY_DONE;
> > > +           goto err_put_fp;
> > >     }
> > >
> > >     ctx->chg.event_type = action;
> > > @@ -2563,6 +2563,9 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
> > >     queue_work(ctx->gdev->line_state_wq, &ctx->work);
> > >
> > >     return NOTIFY_OK;
> > > +err_put_fp:
> > > +   fput(fp);
> > > +   return NOTIFY_DONE;
> > >  }
> > >
> > >  static int gpio_device_unregistered_notify(struct notifier_block *nb,
> > > --
> > > 2.52.0.457.g6b5491de43-goog
> > >
> > >
> >
> > There's only one place where you need this fput(), please do it directly in
> > the error path of kzalloc() and drop the label.
>
> Sure, just wanted to give a heads up: 17/23 might introduce the label back.
>
> v2: https://lore.kernel.org/all/20260120030857.2144847-1-tzungbi@kernel.org

That's alright, we prefer a smaller patch for backporting.

Bart