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

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMRc=MfNHuTYsZJ+_RqPN1TtLOHsenv2neD5wvhA18NH6m7XjA@mail.gmail.com>
Date: Fri, 16 Jan 2026 15:38:37 +0100
From: Bartosz Golaszewski <brgl@...nel.org>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: Tzung-Bi Shih <tzungbi@...nel.org>, Benson Leung <bleung@...omium.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J . Wysocki" <rafael@...nel.org>, 
	Danilo Krummrich <dakr@...nel.org>, Linus Walleij <linusw@...nel.org>, Jonathan Corbet <corbet@....net>, 
	Shuah Khan <shuah@...nel.org>, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, 
	chrome-platform@...ts.linux.dev, linux-kselftest@...r.kernel.org, 
	Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Wolfram Sang <wsa+renesas@...g-engineering.com>, Simona Vetter <simona.vetter@...ll.ch>, 
	Dan Williams <dan.j.williams@...el.com>, linux-gpio@...r.kernel.org, 
	stable@...r.kernel.org
Subject: Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`

On Fri, Jan 16, 2026 at 3:14 PM Jason Gunthorpe <jgg@...dia.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@...r.kernel.org
> > Fixes: c351bb64cbe6 ("gpiolib: free device name on error path to fix kmemleak")
> > Signed-off-by: Tzung-Bi Shih <tzungbi@...nel.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