* [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 13:15 ` Bartosz Golaszewski
2026-01-16 14:13 ` Jason Gunthorpe
2026-01-16 8:10 ` [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register() Tzung-Bi Shih
` (23 subsequent siblings)
24 siblings, 2 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio, stable
`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
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`
2026-01-16 8:10 ` [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name` Tzung-Bi Shih
@ 2026-01-16 13:15 ` Bartosz Golaszewski
2026-01-16 13:27 ` Greg Kroah-Hartman
2026-01-20 4:29 ` Tzung-Bi Shih
2026-01-16 14:13 ` Jason Gunthorpe
1 sibling, 2 replies; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-16 13:15 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, Jason Gunthorpe, linux-gpio, stable,
Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
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
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`
2026-01-16 13:15 ` Bartosz Golaszewski
@ 2026-01-16 13:27 ` Greg Kroah-Hartman
2026-01-16 13:30 ` Bartosz Golaszewski
2026-01-20 4:29 ` Tzung-Bi Shih
1 sibling, 1 reply; 49+ messages in thread
From: Greg Kroah-Hartman @ 2026-01-16 13:27 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Tzung-Bi Shih, Jonathan Corbet, Shuah Khan, linux-doc,
linux-kernel, chrome-platform, linux-kselftest, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio, stable, Benson Leung, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
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
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`
2026-01-16 13:27 ` Greg Kroah-Hartman
@ 2026-01-16 13:30 ` Bartosz Golaszewski
0 siblings, 0 replies; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-16 13:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Tzung-Bi Shih, Jonathan Corbet, Shuah Khan, linux-doc,
linux-kernel, chrome-platform, linux-kselftest, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio, stable, Benson Leung, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`
2026-01-16 13:15 ` Bartosz Golaszewski
2026-01-16 13:27 ` Greg Kroah-Hartman
@ 2026-01-20 4:29 ` Tzung-Bi Shih
1 sibling, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-20 4:29 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, Jason Gunthorpe, linux-gpio, stable,
Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
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.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`
2026-01-16 8:10 ` [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name` Tzung-Bi Shih
2026-01-16 13:15 ` Bartosz Golaszewski
@ 2026-01-16 14:13 ` Jason Gunthorpe
2026-01-16 14:38 ` Bartosz Golaszewski
1 sibling, 1 reply; 49+ messages in thread
From: Jason Gunthorpe @ 2026-01-16 14:13 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij,
Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, linux-gpio, stable
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
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`
2026-01-16 14:13 ` Jason Gunthorpe
@ 2026-01-16 14:38 ` Bartosz Golaszewski
2026-01-20 4:30 ` Tzung-Bi Shih
0 siblings, 1 reply; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-16 14:38 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Tzung-Bi Shih, Benson Leung, Greg Kroah-Hartman,
Rafael J . Wysocki, Danilo Krummrich, Linus Walleij,
Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, linux-gpio, stable
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
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`
2026-01-16 14:38 ` Bartosz Golaszewski
@ 2026-01-20 4:30 ` Tzung-Bi Shih
2026-01-20 9:43 ` Bartosz Golaszewski
0 siblings, 1 reply; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-20 4:30 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Jason Gunthorpe, Benson Leung, Greg Kroah-Hartman,
Rafael J . Wysocki, Danilo Krummrich, Linus Walleij,
Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, linux-gpio, stable
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.
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name`
2026-01-20 4:30 ` Tzung-Bi Shih
@ 2026-01-20 9:43 ` Bartosz Golaszewski
0 siblings, 0 replies; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-20 9:43 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Jason Gunthorpe, Benson Leung, Greg Kroah-Hartman,
Rafael J . Wysocki, Danilo Krummrich, Linus Walleij,
Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, linux-gpio, stable
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name` Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-20 8:50 ` Bartosz Golaszewski
2026-01-16 8:10 ` [PATCH 03/23] gpiolib: Fix resource leaks on errors in gpiochip_add_data_with_key() Tzung-Bi Shih
` (22 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio, stable
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
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()
2026-01-16 8:10 ` [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register() Tzung-Bi Shih
@ 2026-01-20 8:50 ` Bartosz Golaszewski
2026-01-20 9:34 ` Tzung-Bi Shih
0 siblings, 1 reply; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-20 8:50 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij, Jonathan Corbet, Shuah Khan,
linux-doc, linux-kernel, chrome-platform, linux-kselftest,
Laurent Pinchart, Wolfram Sang, Simona Vetter, Dan Williams,
Jason Gunthorpe, linux-gpio, stable
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
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()
2026-01-20 8:50 ` Bartosz Golaszewski
@ 2026-01-20 9:34 ` Tzung-Bi Shih
2026-01-20 9:39 ` Bartosz Golaszewski
0 siblings, 1 reply; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-20 9:34 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij, Jonathan Corbet, Shuah Khan,
linux-doc, linux-kernel, chrome-platform, linux-kselftest,
Laurent Pinchart, Wolfram Sang, Simona Vetter, Dan Williams,
Jason Gunthorpe, linux-gpio, stable
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/
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()
2026-01-20 9:34 ` Tzung-Bi Shih
@ 2026-01-20 9:39 ` Bartosz Golaszewski
0 siblings, 0 replies; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-20 9:39 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij, Jonathan Corbet, Shuah Khan,
linux-doc, linux-kernel, chrome-platform, linux-kselftest,
Laurent Pinchart, Wolfram Sang, Simona Vetter, Dan Williams,
Jason Gunthorpe, linux-gpio, stable
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 03/23] gpiolib: Fix resource leaks on errors in gpiochip_add_data_with_key()
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 01/23] gpiolib: Correct wrong kfree() usage for `kobj->name` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register() Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify() Tzung-Bi Shih
` (21 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio, stable
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
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (2 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 03/23] gpiolib: Fix resource leaks on errors in gpiochip_add_data_with_key() Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 13:26 ` Bartosz Golaszewski
2026-01-16 8:10 ` [PATCH 05/23] gpiolib: cdev: Correct return code on memory allocation failure Tzung-Bi Shih
` (20 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio, stable
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
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()
2026-01-16 8:10 ` [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify() Tzung-Bi Shih
@ 2026-01-16 13:26 ` Bartosz Golaszewski
2026-01-20 3:11 ` Tzung-Bi Shih
0 siblings, 1 reply; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-16 13:26 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, Jason Gunthorpe, linux-gpio, stable,
Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
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
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()
2026-01-16 13:26 ` Bartosz Golaszewski
@ 2026-01-20 3:11 ` Tzung-Bi Shih
2026-01-20 8:49 ` Bartosz Golaszewski
0 siblings, 1 reply; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-20 3:11 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, Jason Gunthorpe, linux-gpio, stable,
Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
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
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()
2026-01-20 3:11 ` Tzung-Bi Shih
@ 2026-01-20 8:49 ` Bartosz Golaszewski
0 siblings, 0 replies; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-20 8:49 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, Jason Gunthorpe, linux-gpio, stable,
Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 05/23] gpiolib: cdev: Correct return code on memory allocation failure
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (3 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 04/23] gpiolib: Fix resource leaks on errors in lineinfo_changed_notify() Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 06/23] gpiolib: Access `gpio_bus_type` in gpiochip_setup_dev() Tzung-Bi Shih
` (19 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
-ENOMEM is a more appropriate return code for memory allocation
failures. Correct it.
Fixes: 20bddcb40b2b ("gpiolib: cdev: replace locking wrappers for gpio_device with guards")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-cdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 6196aab5ed74..66bd260c68e9 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2699,7 +2699,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
if (!cdev)
- return -ENODEV;
+ return -ENOMEM;
cdev->watched_lines = bitmap_zalloc(gdev->ngpio, GFP_KERNEL);
if (!cdev->watched_lines)
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 06/23] gpiolib: Access `gpio_bus_type` in gpiochip_setup_dev()
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (4 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 05/23] gpiolib: cdev: Correct return code on memory allocation failure Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 07/23] gpiolib: Remove redundant check for struct gpio_chip Tzung-Bi Shih
` (18 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
To make the intent clear, access `gpio_bus_type` only when it's ready in
gpiochip_setup_dev().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 6fac59716405..3242644eebc6 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -902,6 +902,8 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
int ret;
+ gdev->dev.bus = &gpio_bus_type;
+
/*
* If fwnode doesn't belong to another device, it's safe to clear its
* initialized flag.
@@ -1096,7 +1098,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
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));
@@ -1217,8 +1218,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
* we get a device node entry in sysfs under
* /sys/bus/gpio/devices/gpiochipN/dev that can be used for
* coldplug of device nodes and other udev business.
- * We can do this only if gpiolib has been initialized.
- * Otherwise, defer until later.
+ * We can do this only if gpiolib has been initialized
+ * (i.e., `gpio_bus_type` is ready). Otherwise, defer until later.
*/
if (gpiolib_initialized) {
ret = gpiochip_setup_dev(gdev);
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 07/23] gpiolib: Remove redundant check for struct gpio_chip
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (5 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 06/23] gpiolib: Access `gpio_bus_type` in gpiochip_setup_dev() Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 08/23] gpiolib: sysfs: " Tzung-Bi Shih
` (17 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
gpiolib_dbg_show() is only called by gpiolib_seq_show() which has
ensured the struct gpio_chip. Remove the redundant check in
gpiolib_dbg_show().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3242644eebc6..42da3bbc5ab8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -5313,23 +5313,14 @@ core_initcall(gpiolib_dev_init);
#ifdef CONFIG_DEBUG_FS
-static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
+static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *gc)
{
bool active_low, is_irq, is_out;
struct gpio_desc *desc;
unsigned int gpio = 0;
- struct gpio_chip *gc;
unsigned long flags;
int value;
- guard(srcu)(&gdev->srcu);
-
- gc = srcu_dereference(gdev->chip, &gdev->srcu);
- if (!gc) {
- seq_puts(s, "Underlying GPIO chip is gone\n");
- return;
- }
-
for_each_gpio_desc(gc, desc) {
guard(srcu)(&desc->gdev->desc_srcu);
flags = READ_ONCE(desc->flags);
@@ -5442,7 +5433,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
if (gc->dbg_show)
gc->dbg_show(s, gc);
else
- gpiolib_dbg_show(s, gdev);
+ gpiolib_dbg_show(s, gc);
return 0;
}
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 08/23] gpiolib: sysfs: Remove redundant check for struct gpio_chip
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (6 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 07/23] gpiolib: Remove redundant check for struct gpio_chip Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 09/23] gpiolib: Ensure struct gpio_chip for gpiochip_setup_dev() Tzung-Bi Shih
` (16 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
gpiochip_sysfs_unregister() is only called by gpiochip_remove() where
the struct gpio_chip is ensured.
Remove the redundant check.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-sysfs.c | 9 +--------
drivers/gpio/gpiolib-sysfs.h | 6 ++++--
drivers/gpio/gpiolib.c | 2 +-
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index cd553acf3055..8e6b09d8b559 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -1048,11 +1048,10 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
return 0;
}
-void gpiochip_sysfs_unregister(struct gpio_device *gdev)
+void gpiochip_sysfs_unregister(struct gpio_device *gdev, struct gpio_chip *chip)
{
struct gpiodev_data *data;
struct gpio_desc *desc;
- struct gpio_chip *chip;
scoped_guard(mutex, &sysfs_lock) {
data = gdev_get_data(gdev);
@@ -1066,12 +1065,6 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
kfree(data);
}
- guard(srcu)(&gdev->srcu);
-
- chip = srcu_dereference(gdev->chip, &gdev->srcu);
- if (!chip)
- return;
-
/* unregister gpiod class devices owned by sysfs */
for_each_gpio_desc_with_flag(chip, desc, GPIOD_FLAG_SYSFS) {
gpiod_unexport(desc);
diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
index b794b396d6a5..93debe8e118c 100644
--- a/drivers/gpio/gpiolib-sysfs.h
+++ b/drivers/gpio/gpiolib-sysfs.h
@@ -8,7 +8,8 @@ struct gpio_device;
#ifdef CONFIG_GPIO_SYSFS
int gpiochip_sysfs_register(struct gpio_device *gdev);
-void gpiochip_sysfs_unregister(struct gpio_device *gdev);
+void gpiochip_sysfs_unregister(struct gpio_device *gdev,
+ struct gpio_chip *chip);
#else
@@ -17,7 +18,8 @@ static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
return 0;
}
-static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev)
+static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev,
+ struct gpio_chip *chip)
{
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 42da3bbc5ab8..c3e1465042c4 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1286,7 +1286,7 @@ void gpiochip_remove(struct gpio_chip *gc)
struct gpio_device *gdev = gc->gpiodev;
/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
- gpiochip_sysfs_unregister(gdev);
+ gpiochip_sysfs_unregister(gdev, gc);
gpiochip_free_hogs(gc);
gpiochip_free_remaining_irqs(gc);
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 09/23] gpiolib: Ensure struct gpio_chip for gpiochip_setup_dev()
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (7 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 08/23] gpiolib: sysfs: " Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 10/23] gpiolib: cdev: Don't check struct gpio_chip in gpio_chrdev_open() Tzung-Bi Shih
` (15 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Ensure struct gpio_chip for gpiochip_setup_dev(). This eliminates a few
checks for struct gpio_chip.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-cdev.c | 13 ++-----------
drivers/gpio/gpiolib-cdev.h | 3 ++-
drivers/gpio/gpiolib-sysfs.c | 11 ++---------
drivers/gpio/gpiolib-sysfs.h | 5 +++--
drivers/gpio/gpiolib.c | 24 +++++++++++++++++-------
5 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 66bd260c68e9..24449bbe38c9 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2784,9 +2784,9 @@ static const struct file_operations gpio_fileops = {
#endif
};
-int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
+int gpiolib_cdev_register(struct gpio_device *gdev, struct gpio_chip *gc,
+ dev_t devt)
{
- struct gpio_chip *gc;
int ret;
cdev_init(&gdev->chrdev, &gpio_fileops);
@@ -2802,18 +2802,9 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
if (ret)
goto err_free_workqueue;
- guard(srcu)(&gdev->srcu);
- gc = srcu_dereference(gdev->chip, &gdev->srcu);
- 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;
diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
index b42644cbffb8..16ef1e2e96a0 100644
--- a/drivers/gpio/gpiolib-cdev.h
+++ b/drivers/gpio/gpiolib-cdev.h
@@ -7,7 +7,8 @@
struct gpio_device;
-int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt);
+int gpiolib_cdev_register(struct gpio_device *gdev, struct gpio_chip *gc,
+ dev_t devt);
void gpiolib_cdev_unregister(struct gpio_device *gdev);
#endif /* GPIOLIB_CDEV_H */
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 8e6b09d8b559..a4427a5cfa85 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -978,10 +978,9 @@ void gpiod_unexport(struct gpio_desc *desc)
}
EXPORT_SYMBOL_GPL(gpiod_unexport);
-int gpiochip_sysfs_register(struct gpio_device *gdev)
+int gpiochip_sysfs_register(struct gpio_device *gdev, struct gpio_chip *chip)
{
struct gpiodev_data *data;
- struct gpio_chip *chip;
struct device *parent;
int err;
@@ -994,12 +993,6 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
if (!class_is_registered(&gpio_class))
return 0;
- guard(srcu)(&gdev->srcu);
-
- chip = srcu_dereference(gdev->chip, &gdev->srcu);
- if (!chip)
- return -ENODEV;
-
/*
* For sysfs backward compatibility we need to preserve this
* preferred parenting to the gpio_chip parent field, if set.
@@ -1082,7 +1075,7 @@ static int gpiofind_sysfs_register(struct gpio_chip *gc, const void *data)
struct gpio_device *gdev = gc->gpiodev;
int ret;
- ret = gpiochip_sysfs_register(gdev);
+ ret = gpiochip_sysfs_register(gdev, gc);
if (ret)
gpiochip_err(gc, "failed to register the sysfs entry: %d\n", ret);
diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
index 93debe8e118c..192b1ee041a6 100644
--- a/drivers/gpio/gpiolib-sysfs.h
+++ b/drivers/gpio/gpiolib-sysfs.h
@@ -7,13 +7,14 @@ struct gpio_device;
#ifdef CONFIG_GPIO_SYSFS
-int gpiochip_sysfs_register(struct gpio_device *gdev);
+int gpiochip_sysfs_register(struct gpio_device *gdev, struct gpio_chip *chip);
void gpiochip_sysfs_unregister(struct gpio_device *gdev,
struct gpio_chip *chip);
#else
-static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
+static inline int gpiochip_sysfs_register(struct gpio_device *gdev,
+ struct gpio_chip *chip)
{
return 0;
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c3e1465042c4..efe72b81e131 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -882,14 +882,15 @@ static const struct device_type gpio_dev_type = {
};
#ifdef CONFIG_GPIO_CDEV
-#define gcdev_register(gdev, devt) gpiolib_cdev_register((gdev), (devt))
+#define gcdev_register(gdev, gc, devt) \
+ gpiolib_cdev_register((gdev), (gc), (devt))
#define gcdev_unregister(gdev) gpiolib_cdev_unregister((gdev))
#else
/*
* gpiolib_cdev_register() indirectly calls device_add(), which is still
* required even when cdev is not selected.
*/
-#define gcdev_register(gdev, devt) device_add(&(gdev)->dev)
+#define gcdev_register(gdev, gc, devt) device_add(&(gdev)->dev)
#define gcdev_unregister(gdev) device_del(&(gdev)->dev)
#endif
@@ -897,7 +898,7 @@ static const struct device_type gpio_dev_type = {
* 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)
+static int gpiochip_setup_dev(struct gpio_device *gdev, struct gpio_chip *gc)
{
struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
int ret;
@@ -911,11 +912,11 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
if (fwnode && !fwnode->dev)
fwnode_dev_initialized(fwnode, false);
- ret = gcdev_register(gdev, gpio_devt);
+ ret = gcdev_register(gdev, gc, gpio_devt);
if (ret)
return ret;
- ret = gpiochip_sysfs_register(gdev);
+ ret = gpiochip_sysfs_register(gdev, gc);
if (ret)
goto err_remove_device;
@@ -962,13 +963,22 @@ static void machine_gpiochip_add(struct gpio_chip *gc)
static void gpiochip_setup_devs(void)
{
struct gpio_device *gdev;
+ struct gpio_chip *gc;
int ret;
guard(srcu)(&gpio_devices_srcu);
list_for_each_entry_srcu(gdev, &gpio_devices, list,
srcu_read_lock_held(&gpio_devices_srcu)) {
- ret = gpiochip_setup_dev(gdev);
+ guard(srcu)(&gdev->srcu);
+
+ gc = srcu_dereference(gdev->chip, &gdev->srcu);
+ if (!gc) {
+ dev_err(&gdev->dev, "Underlying GPIO chip is gone\n");
+ continue;
+ }
+
+ ret = gpiochip_setup_dev(gdev, gc);
if (ret) {
gpio_device_put(gdev);
dev_err(&gdev->dev,
@@ -1222,7 +1232,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
* (i.e., `gpio_bus_type` is ready). Otherwise, defer until later.
*/
if (gpiolib_initialized) {
- ret = gpiochip_setup_dev(gdev);
+ ret = gpiochip_setup_dev(gdev, gc);
if (ret)
goto err_teardown_shared;
}
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 10/23] gpiolib: cdev: Don't check struct gpio_chip in gpio_chrdev_open()
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (8 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 09/23] gpiolib: Ensure struct gpio_chip for gpiochip_setup_dev() Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 11/23] selftests: gpio: Add gpio-cdev-uaf tests Tzung-Bi Shih
` (14 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
It's harmless even if: chrdev_open() and cdev_device_del() run at the
same time, and gpio_chrdev_open() gets called after the underlying GPIO
chip has gone. The subsequent file operations check the availability
of struct gpio_chip anyway.
Don't check struct gpio_chip in gpio_chrdev_open().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-cdev.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 24449bbe38c9..e42cfdb47885 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2689,13 +2689,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
struct gpio_device *gdev = container_of(inode->i_cdev,
struct gpio_device, chrdev);
struct gpio_chardev_data *cdev;
- int ret = -ENOMEM;
-
- guard(srcu)(&gdev->srcu);
-
- /* Fail on open if the backing gpiochip is gone */
- if (!rcu_access_pointer(gdev->chip))
- return -ENODEV;
+ int ret;
cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
if (!cdev)
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 11/23] selftests: gpio: Add gpio-cdev-uaf tests
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (9 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 10/23] gpiolib: cdev: Don't check struct gpio_chip in gpio_chrdev_open() Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 12/23] gpiolib: Add revocable provider handle for struct gpio_chip Tzung-Bi Shih
` (13 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Add tests for gpiolib-cdev to make sure accessing to dangling resources
via the opening FD won't crash the system after the underlying resource
providers have gone.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
tools/testing/selftests/gpio/Makefile | 5 +-
tools/testing/selftests/gpio/gpio-cdev-uaf.c | 320 ++++++++++++++++++
tools/testing/selftests/gpio/gpio-cdev-uaf.sh | 67 ++++
3 files changed, 390 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/gpio/gpio-cdev-uaf.c
create mode 100755 tools/testing/selftests/gpio/gpio-cdev-uaf.sh
diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index 7bfe315f7001..741ab21e1260 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,8 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
-TEST_PROGS := gpio-mockup.sh gpio-sim.sh gpio-aggregator.sh
+TEST_PROGS := gpio-mockup.sh gpio-sim.sh gpio-aggregator.sh gpio-cdev-uaf.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name \
+ gpio-cdev-uaf
CFLAGS += -O2 -g -Wall $(KHDR_INCLUDES)
include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-cdev-uaf.c b/tools/testing/selftests/gpio/gpio-cdev-uaf.c
new file mode 100644
index 000000000000..28c651998279
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-cdev-uaf.c
@@ -0,0 +1,320 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for UAF tests.
+ *
+ * Copyright 2026 Google LLC
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/gpio.h>
+#include <poll.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#define CONFIGFS_DIR "/sys/kernel/config/gpio-sim"
+#define PROCFS_DIR "/proc"
+
+static void print_usage(void)
+{
+ printf("usage:\n");
+ printf(" gpio-cdev-uaf [chip|handle|event|req] "
+ "[poll|read|ioctl|fdinfo]\n");
+}
+
+static int _create_chip(const char *name, int create)
+{
+ char path[64];
+
+ snprintf(path, sizeof(path), CONFIGFS_DIR "/%s", name);
+
+ if (create)
+ return mkdir(path, 0755);
+ else
+ return rmdir(path);
+}
+
+static int create_chip(const char *name)
+{
+ return _create_chip(name, 1);
+}
+
+static void remove_chip(const char *name)
+{
+ _create_chip(name, 0);
+}
+
+static int _create_bank(const char *chip_name, const char *name, int create)
+{
+ char path[64];
+
+ snprintf(path, sizeof(path), CONFIGFS_DIR "/%s/%s", chip_name, name);
+
+ if (create)
+ return mkdir(path, 0755);
+ else
+ return rmdir(path);
+}
+
+static int create_bank(const char *chip_name, const char *name)
+{
+ return _create_bank(chip_name, name, 1);
+}
+
+static void remove_bank(const char *chip_name, const char *name)
+{
+ _create_bank(chip_name, name, 0);
+}
+
+static int _enable_chip(const char *name, int enable)
+{
+ char path[64];
+ int fd, ret;
+
+ snprintf(path, sizeof(path), CONFIGFS_DIR "/%s/live", name);
+
+ fd = open(path, O_WRONLY);
+ if (fd == -1)
+ return fd;
+
+ if (enable)
+ ret = write(fd, "1", 1);
+ else
+ ret = write(fd, "0", 1);
+
+ close(fd);
+ return ret == 1 ? 0 : -1;
+}
+
+static int enable_chip(const char *name)
+{
+ return _enable_chip(name, 1);
+}
+
+static void disable_chip(const char *name)
+{
+ _enable_chip(name, 0);
+}
+
+static int open_chip(const char *chip_name, const char *bank_name)
+{
+ char path[64], dev_name[32];
+ int ret, fd;
+
+ ret = create_chip(chip_name);
+ if (ret) {
+ fprintf(stderr, "failed to create chip\n");
+ return ret;
+ }
+
+ ret = create_bank(chip_name, bank_name);
+ if (ret) {
+ fprintf(stderr, "failed to create bank\n");
+ goto err_remove_chip;
+ }
+
+ ret = enable_chip(chip_name);
+ if (ret) {
+ fprintf(stderr, "failed to enable chip\n");
+ goto err_remove_bank;
+ }
+
+ snprintf(path, sizeof(path), CONFIGFS_DIR "/%s/%s/chip_name",
+ chip_name, bank_name);
+
+ fd = open(path, O_RDONLY);
+ if (fd == -1) {
+ ret = fd;
+ fprintf(stderr, "failed to open %s\n", path);
+ goto err_disable_chip;
+ }
+
+ ret = read(fd, dev_name, sizeof(dev_name) - 1);
+ close(fd);
+ if (ret == -1) {
+ fprintf(stderr, "failed to read %s\n", path);
+ goto err_disable_chip;
+ }
+ dev_name[ret] = '\0';
+ if (ret && dev_name[ret - 1] == '\n')
+ dev_name[ret - 1] = '\0';
+
+ snprintf(path, sizeof(path), "/dev/%s", dev_name);
+
+ fd = open(path, O_RDWR);
+ if (fd == -1) {
+ ret = fd;
+ fprintf(stderr, "failed to open %s\n", path);
+ goto err_disable_chip;
+ }
+
+ return fd;
+err_disable_chip:
+ disable_chip(chip_name);
+err_remove_bank:
+ remove_bank(chip_name, bank_name);
+err_remove_chip:
+ remove_chip(chip_name);
+ return ret;
+}
+
+static void close_chip(const char *chip_name, const char *bank_name)
+{
+ disable_chip(chip_name);
+ remove_bank(chip_name, bank_name);
+ remove_chip(chip_name);
+}
+
+static int test_poll(int fd)
+{
+ struct pollfd pfds;
+
+ pfds.fd = fd;
+ pfds.events = POLLIN;
+ pfds.revents = 0;
+
+ if (poll(&pfds, 1, 0) == -1)
+ return -1;
+
+ return (pfds.revents & ~(POLLHUP | POLLERR)) ? -1 : 0;
+}
+
+static int test_read(int fd)
+{
+ char data;
+
+ if (read(fd, &data, 1) == -1 && errno == ENODEV)
+ return 0;
+ return -1;
+}
+
+static int test_ioctl(int fd)
+{
+ if (ioctl(fd, 0, NULL) == -1 && errno == ENODEV)
+ return 0;
+ return -1;
+}
+
+static int test_fdinfo(int fd)
+{
+ char path[64], fdinfo[128];
+ int pfd, ret;
+
+ snprintf(path, sizeof(path), PROCFS_DIR "/%d/fdinfo/%d", getpid(), fd);
+
+ pfd = open(path, O_RDONLY);
+ if (pfd == -1)
+ return -1;
+
+ ret = read(pfd, fdinfo, sizeof(fdinfo));
+ close(pfd);
+ if (ret == -1)
+ return -1;
+
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ int cfd, fd, ret;
+ int (*test_func)(int);
+
+ if (argc != 3) {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ if (strcmp(argv[1], "chip") == 0 || strcmp(argv[1], "event") == 0) {
+ if (strcmp(argv[2], "poll") &&
+ strcmp(argv[2], "read") &&
+ strcmp(argv[2], "ioctl")) {
+ fprintf(stderr, "unknown command: %s\n", argv[2]);
+ return EXIT_FAILURE;
+ }
+ } else if (strcmp(argv[1], "req") == 0) {
+ if (strcmp(argv[2], "poll") &&
+ strcmp(argv[2], "read") &&
+ strcmp(argv[2], "ioctl") &&
+ strcmp(argv[2], "fdinfo")) {
+ fprintf(stderr, "unknown command: %s\n", argv[2]);
+ return EXIT_FAILURE;
+ }
+ } else if (strcmp(argv[1], "handle") == 0) {
+ if (strcmp(argv[2], "ioctl")) {
+ fprintf(stderr, "unknown command: %s\n", argv[2]);
+ return EXIT_FAILURE;
+ }
+ } else {
+ fprintf(stderr, "unknown command: %s\n", argv[1]);
+ return EXIT_FAILURE;
+ }
+
+ if (strcmp(argv[2], "poll") == 0)
+ test_func = test_poll;
+ else if (strcmp(argv[2], "read") == 0)
+ test_func = test_read;
+ else if (strcmp(argv[2], "ioctl") == 0)
+ test_func = test_ioctl;
+ else /* strcmp(argv[2], "fdinfo") == 0 */
+ test_func = test_fdinfo;
+
+ cfd = open_chip("chip", "bank");
+ if (cfd == -1) {
+ fprintf(stderr, "failed to open chip\n");
+ return EXIT_FAILURE;
+ }
+
+ /* Step 1: Hold a FD to the test target. */
+ if (strcmp(argv[1], "chip") == 0) {
+ fd = cfd;
+ } else if (strcmp(argv[1], "handle") == 0) {
+ struct gpiohandle_request req = {0};
+
+ req.lines = 1;
+ if (ioctl(cfd, GPIO_GET_LINEHANDLE_IOCTL, &req) == -1) {
+ fprintf(stderr, "failed to get handle FD\n");
+ goto err_close_chip;
+ }
+
+ close(cfd);
+ fd = req.fd;
+ } else if (strcmp(argv[1], "event") == 0) {
+ struct gpioevent_request req = {0};
+
+ if (ioctl(cfd, GPIO_GET_LINEEVENT_IOCTL, &req) == -1) {
+ fprintf(stderr, "failed to get event FD\n");
+ goto err_close_chip;
+ }
+
+ close(cfd);
+ fd = req.fd;
+ } else { /* strcmp(argv[1], "req") == 0 */
+ struct gpio_v2_line_request req = {0};
+
+ req.num_lines = 1;
+ if (ioctl(cfd, GPIO_V2_GET_LINE_IOCTL, &req) == -1) {
+ fprintf(stderr, "failed to get req FD\n");
+ goto err_close_chip;
+ }
+
+ close(cfd);
+ fd = req.fd;
+ }
+
+ /* Step 2: Free the chip. */
+ close_chip("chip", "bank");
+
+ /* Step 3: Access the dangling FD to trigger UAF. */
+ ret = test_func(fd);
+ close(fd);
+ return ret ? EXIT_FAILURE : EXIT_SUCCESS;
+err_close_chip:
+ close(cfd);
+ close_chip("chip", "bank");
+ return EXIT_FAILURE;
+}
diff --git a/tools/testing/selftests/gpio/gpio-cdev-uaf.sh b/tools/testing/selftests/gpio/gpio-cdev-uaf.sh
new file mode 100755
index 000000000000..910bcdb14841
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-cdev-uaf.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2026 Google LLC
+
+BASE_DIR=`dirname $0`
+MODULE="gpio-cdev-uaf"
+
+fail() {
+ echo "$*" >&2
+ echo "GPIO $MODULE test FAIL"
+ exit 1
+}
+
+skip() {
+ echo "$*" >&2
+ echo "GPIO $MODULE test SKIP"
+ exit 4
+}
+
+# Load the gpio-sim module. This will pull in configfs if needed too.
+modprobe gpio-sim || skip "unable to load the gpio-sim module"
+# Make sure configfs is mounted at /sys/kernel/config. Wait a bit if needed.
+for _ in `seq 5`; do
+ mountpoint -q /sys/kernel/config && break
+ mount -t configfs none /sys/kernel/config
+ sleep 0.1
+done
+mountpoint -q /sys/kernel/config || \
+ skip "configfs not mounted at /sys/kernel/config"
+
+echo "1. GPIO"
+
+echo "1.1. poll"
+$BASE_DIR/gpio-cdev-uaf chip poll || fail "failed to test chip poll"
+echo "1.2. read"
+$BASE_DIR/gpio-cdev-uaf chip read || fail "failed to test chip read"
+echo "1.3. ioctl"
+$BASE_DIR/gpio-cdev-uaf chip ioctl || fail "failed to test chip ioctl"
+
+echo "2. linehandle"
+
+echo "2.1. ioctl"
+$BASE_DIR/gpio-cdev-uaf handle ioctl || fail "failed to test handle ioctl"
+
+echo "3. lineevent"
+
+echo "3.1. read"
+$BASE_DIR/gpio-cdev-uaf event read || fail "failed to test event read"
+echo "3.2. poll"
+$BASE_DIR/gpio-cdev-uaf event poll || fail "failed to test event poll"
+echo "3.3. ioctl"
+$BASE_DIR/gpio-cdev-uaf event ioctl || fail "failed to test event ioctl"
+
+echo "4. linereq"
+
+echo "4.1. read"
+$BASE_DIR/gpio-cdev-uaf req read || fail "failed to test req read"
+echo "4.2. poll"
+$BASE_DIR/gpio-cdev-uaf req poll || fail "failed to test req poll"
+echo "4.3. ioctl"
+$BASE_DIR/gpio-cdev-uaf req ioctl || fail "failed to test req ioctl"
+echo "4.4. fdinfo"
+if mountpoint -q /proc; then
+ $BASE_DIR/gpio-cdev-uaf req fdinfo || fail "failed to test req fdinfo"
+fi
+
+echo "GPIO $MODULE test PASS"
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 12/23] gpiolib: Add revocable provider handle for struct gpio_chip
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (10 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 11/23] selftests: gpio: Add gpio-cdev-uaf tests Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 13/23] gpiolib: cdev: Leverage revocable for gpio_fileops Tzung-Bi Shih
` (12 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
The underlying chip can be removed asynchronously. `gdev->srcu` is used
to ensure the synchronization before accessing `gdev->chip`.
Revocable encapsulates the details in a neat way. Add revocable
provider handle for the corresponding struct gpio_chip in struct
gpio_device so that we can start to hide the synchronization details.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib.c | 19 +++++++++++++++++++
drivers/gpio/gpiolib.h | 2 ++
2 files changed, 21 insertions(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index efe72b81e131..6226dc738281 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -22,6 +22,7 @@
#include <linux/nospec.h>
#include <linux/of.h>
#include <linux/pinctrl/consumer.h>
+#include <linux/revocable.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/srcu.h>
@@ -1105,6 +1106,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (ret)
goto err_cleanup_gdev_srcu;
+ gdev->chip_rp = revocable_provider_alloc(gc);
+ if (!gdev->chip_rp) {
+ ret = -ENOMEM;
+ goto err_cleanup_desc_srcu;
+ }
+
device_initialize(&gdev->dev);
/* From this point, the .release() function cleans up gdev->dev */
gdev->dev.type = &gpio_dev_type;
@@ -1258,9 +1265,20 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
list_del_rcu(&gdev->list);
synchronize_srcu(&gpio_devices_srcu);
err_put_device:
+ /*
+ * Error handling of the revocable provider is tricky. Unlike other
+ * allocated resources for `gdev` are freed in gpiodev_release().
+ * We need to call revocable_provider_revoke() here as it's designed
+ * to be called when the chip is gone (i.e. gpiochip_remove()).
+ *
+ * Note: must before gpio_device_put() as it frees `gdev`.
+ */
+ revocable_provider_revoke(gdev->chip_rp);
gpio_device_put(gdev);
goto err_print_message;
+err_cleanup_desc_srcu:
+ cleanup_srcu_struct(&gdev->desc_srcu);
err_cleanup_gdev_srcu:
cleanup_srcu_struct(&gdev->srcu);
err_free_label:
@@ -1307,6 +1325,7 @@ void gpiochip_remove(struct gpio_chip *gc)
/* Numb the device, cancelling all outstanding operations */
rcu_assign_pointer(gdev->chip, NULL);
synchronize_srcu(&gdev->srcu);
+ revocable_provider_revoke(gdev->chip_rp);
gpio_device_teardown_shared(gdev);
gpiochip_irqchip_remove(gc);
acpi_gpiochip_remove(gc);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 77f6f2936dc2..e61db3a75e84 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -52,6 +52,7 @@
* @device_notifier: used to notify character device wait queues about the GPIO
* device being unregistered
* @srcu: protects the pointer to the underlying GPIO chip
+ * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
* @pin_ranges: range of pins served by the GPIO driver
*
* This state container holds most of the runtime variable data
@@ -79,6 +80,7 @@ struct gpio_device {
struct workqueue_struct *line_state_wq;
struct blocking_notifier_head device_notifier;
struct srcu_struct srcu;
+ struct revocable_provider *chip_rp;
#ifdef CONFIG_PINCTRL
/*
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 13/23] gpiolib: cdev: Leverage revocable for gpio_fileops
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (11 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 12/23] gpiolib: Add revocable provider handle for struct gpio_chip Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 14/23] gpiolib: cdev: Leverage revocable for linehandle_fileops Tzung-Bi Shih
` (11 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Struct gpio_device now provides a revocable provider to the underlying
struct gpio_chip. Leverage revocable for gpio_fileops so that it doesn't
need to handle the synchronization by accessing the SRCU explicitly.
Also, it's unneeded to hold a reference count to the struct gpio_device
while the file is opening. The struct gpio_device
(i.e., (struct gpio_chip *)->gpiodev)) is valid as long as struct
gpio_chip is valid.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-cdev.c | 87 ++++++++++++++++++++++---------------
1 file changed, 52 insertions(+), 35 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index e42cfdb47885..832a542c4f7a 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -22,6 +22,7 @@
#include <linux/overflow.h>
#include <linux/pinctrl/consumer.h>
#include <linux/poll.h>
+#include <linux/revocable.h>
#include <linux/seq_file.h>
#include <linux/spinlock.h>
#include <linux/string.h>
@@ -2297,7 +2298,7 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
}
struct gpio_chardev_data {
- struct gpio_device *gdev;
+ struct revocable *chip_rev;
wait_queue_head_t wait;
DECLARE_KFIFO(events, struct gpio_v2_line_info_changed, 32);
struct notifier_block lineinfo_changed_nb;
@@ -2309,9 +2310,8 @@ struct gpio_chardev_data {
struct file *fp;
};
-static int chipinfo_get(struct gpio_chardev_data *cdev, void __user *ip)
+static int chipinfo_get(struct gpio_device *gdev, void __user *ip)
{
- struct gpio_device *gdev = cdev->gdev;
struct gpiochip_info chipinfo;
memset(&chipinfo, 0, sizeof(chipinfo));
@@ -2339,7 +2339,8 @@ static int lineinfo_ensure_abi_version(struct gpio_chardev_data *cdata,
return abiv;
}
-static int lineinfo_get_v1(struct gpio_chardev_data *cdev, void __user *ip,
+static int lineinfo_get_v1(struct gpio_chardev_data *cdev,
+ struct gpio_device *gdev, void __user *ip,
bool watch)
{
struct gpio_desc *desc;
@@ -2350,7 +2351,7 @@ static int lineinfo_get_v1(struct gpio_chardev_data *cdev, void __user *ip,
return -EFAULT;
/* this doubles as a range check on line_offset */
- desc = gpio_device_get_desc(cdev->gdev, lineinfo.line_offset);
+ desc = gpio_device_get_desc(gdev, lineinfo.line_offset);
if (IS_ERR(desc))
return PTR_ERR(desc);
@@ -2375,7 +2376,8 @@ static int lineinfo_get_v1(struct gpio_chardev_data *cdev, void __user *ip,
}
#endif
-static int lineinfo_get(struct gpio_chardev_data *cdev, void __user *ip,
+static int lineinfo_get(struct gpio_chardev_data *cdev,
+ struct gpio_device *gdev, void __user *ip,
bool watch)
{
struct gpio_desc *desc;
@@ -2387,7 +2389,7 @@ static int lineinfo_get(struct gpio_chardev_data *cdev, void __user *ip,
if (!mem_is_zero(lineinfo.padding, sizeof(lineinfo.padding)))
return -EINVAL;
- desc = gpio_device_get_desc(cdev->gdev, lineinfo.offset);
+ desc = gpio_device_get_desc(gdev, lineinfo.offset);
if (IS_ERR(desc))
return PTR_ERR(desc);
@@ -2410,14 +2412,15 @@ static int lineinfo_get(struct gpio_chardev_data *cdev, void __user *ip,
return 0;
}
-static int lineinfo_unwatch(struct gpio_chardev_data *cdev, void __user *ip)
+static int lineinfo_unwatch(struct gpio_chardev_data *cdev,
+ struct gpio_device *gdev, void __user *ip)
{
__u32 offset;
if (copy_from_user(&offset, ip, sizeof(offset)))
return -EFAULT;
- if (offset >= cdev->gdev->ngpio)
+ if (offset >= gdev->ngpio)
return -EINVAL;
if (!test_and_clear_bit(offset, cdev->watched_lines))
@@ -2432,37 +2435,38 @@ static int lineinfo_unwatch(struct gpio_chardev_data *cdev, void __user *ip)
static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
struct gpio_chardev_data *cdev = file->private_data;
- struct gpio_device *gdev = cdev->gdev;
+ struct gpio_chip *gc;
+ struct gpio_device *gdev;
void __user *ip = (void __user *)arg;
- guard(srcu)(&gdev->srcu);
-
/* We fail any subsequent ioctl():s when the chip is gone */
- if (!rcu_access_pointer(gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(cdev->chip_rev, gc);
+ if (!gc)
return -ENODEV;
+ gdev = gc->gpiodev;
/* Fill in the struct and pass to userspace */
switch (cmd) {
case GPIO_GET_CHIPINFO_IOCTL:
- return chipinfo_get(cdev, ip);
+ return chipinfo_get(gdev, ip);
#ifdef CONFIG_GPIO_CDEV_V1
case GPIO_GET_LINEHANDLE_IOCTL:
return linehandle_create(gdev, ip);
case GPIO_GET_LINEEVENT_IOCTL:
return lineevent_create(gdev, ip);
case GPIO_GET_LINEINFO_IOCTL:
- return lineinfo_get_v1(cdev, ip, false);
+ return lineinfo_get_v1(cdev, gdev, ip, false);
case GPIO_GET_LINEINFO_WATCH_IOCTL:
- return lineinfo_get_v1(cdev, ip, true);
+ return lineinfo_get_v1(cdev, gdev, ip, true);
#endif /* CONFIG_GPIO_CDEV_V1 */
case GPIO_V2_GET_LINEINFO_IOCTL:
- return lineinfo_get(cdev, ip, false);
+ return lineinfo_get(cdev, gdev, ip, false);
case GPIO_V2_GET_LINEINFO_WATCH_IOCTL:
- return lineinfo_get(cdev, ip, true);
+ return lineinfo_get(cdev, gdev, ip, true);
case GPIO_V2_GET_LINE_IOCTL:
return linereq_create(gdev, ip);
case GPIO_GET_LINEINFO_UNWATCH_IOCTL:
- return lineinfo_unwatch(cdev, ip);
+ return lineinfo_unwatch(cdev, gdev, ip);
default:
return -EINVAL;
}
@@ -2585,10 +2589,10 @@ static __poll_t lineinfo_watch_poll(struct file *file,
{
struct gpio_chardev_data *cdev = file->private_data;
__poll_t events = 0;
+ struct gpio_chip *gc;
- guard(srcu)(&cdev->gdev->srcu);
-
- if (!rcu_access_pointer(cdev->gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(cdev->chip_rev, gc);
+ if (!gc)
return EPOLLHUP | EPOLLERR;
poll_wait(file, &cdev->wait, pollt);
@@ -2608,10 +2612,10 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
ssize_t bytes_read = 0;
int ret;
size_t event_size;
+ struct gpio_chip *gc;
- guard(srcu)(&cdev->gdev->srcu);
-
- if (!rcu_access_pointer(cdev->gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(cdev->chip_rev, gc);
+ if (!gc)
return -ENODEV;
#ifndef CONFIG_GPIO_CDEV_V1
@@ -2695,13 +2699,16 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
if (!cdev)
return -ENOMEM;
+ cdev->chip_rev = revocable_alloc(gdev->chip_rp);
+ if (!cdev->chip_rev)
+ goto out_free_cdev;
+
cdev->watched_lines = bitmap_zalloc(gdev->ngpio, GFP_KERNEL);
if (!cdev->watched_lines)
- goto out_free_cdev;
+ goto out_free_chip_rev;
init_waitqueue_head(&cdev->wait);
INIT_KFIFO(cdev->events);
- cdev->gdev = gpio_device_get(gdev);
cdev->lineinfo_changed_nb.notifier_call = lineinfo_changed_notify;
scoped_guard(write_lock_irqsave, &gdev->line_state_lock)
@@ -2734,8 +2741,9 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
raw_notifier_chain_unregister(&gdev->line_state_notifier,
&cdev->lineinfo_changed_nb);
out_free_bitmap:
- gpio_device_put(gdev);
bitmap_free(cdev->watched_lines);
+out_free_chip_rev:
+ revocable_free(cdev->chip_rev);
out_free_cdev:
kfree(cdev);
return ret;
@@ -2752,15 +2760,24 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
static int gpio_chrdev_release(struct inode *inode, struct file *file)
{
struct gpio_chardev_data *cdev = file->private_data;
- struct gpio_device *gdev = cdev->gdev;
+ struct gpio_chip *gc;
+ struct gpio_device *gdev;
+
+ REVOCABLE_TRY_ACCESS_SCOPED(cdev->chip_rev, gc) {
+ if (!gc)
+ break;
+ gdev = gc->gpiodev;
+
+ blocking_notifier_chain_unregister(&gdev->device_notifier,
+ &cdev->device_unregistered_nb);
+
+ scoped_guard(write_lock_irqsave, &gdev->line_state_lock)
+ raw_notifier_chain_unregister(&gdev->line_state_notifier,
+ &cdev->lineinfo_changed_nb);
+ }
+ revocable_free(cdev->chip_rev);
- blocking_notifier_chain_unregister(&gdev->device_notifier,
- &cdev->device_unregistered_nb);
- scoped_guard(write_lock_irqsave, &gdev->line_state_lock)
- raw_notifier_chain_unregister(&gdev->line_state_notifier,
- &cdev->lineinfo_changed_nb);
bitmap_free(cdev->watched_lines);
- gpio_device_put(gdev);
kfree(cdev);
return 0;
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 14/23] gpiolib: cdev: Leverage revocable for linehandle_fileops
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (12 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 13/23] gpiolib: cdev: Leverage revocable for gpio_fileops Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 15/23] gpiolib: cdev: Leverage revocable for line_fileops Tzung-Bi Shih
` (10 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Struct gpio_device now provides a revocable provider to the underlying
struct gpio_chip. Leverage revocable for linehandle_fileops so that it
doesn't need to handle the synchronization by accessing the SRCU
explicitly.
Also, it's unneeded to hold a reference count to the struct gpio_device
while the file is opening. The struct gpio_device
(i.e., (struct gpio_chip *)->gpiodev)) is valid as long as struct
gpio_chip is valid.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-cdev.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 832a542c4f7a..f7c6f1367235 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -67,13 +67,13 @@ static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_values), 8));
#ifdef CONFIG_GPIO_CDEV_V1
/**
* struct linehandle_state - contains the state of a userspace handle
- * @gdev: the GPIO device the handle pertains to
+ * @chip_rev: revocable consumer handle for the corresponding struct gpio_chip
* @label: consumer label used to tag descriptors
* @descs: the GPIO descriptors held by this handle
* @num_descs: the number of descriptors held in the descs array
*/
struct linehandle_state {
- struct gpio_device *gdev;
+ struct revocable *chip_rev;
const char *label;
struct gpio_desc *descs[GPIOHANDLES_MAX];
u32 num_descs;
@@ -211,10 +211,10 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd,
DECLARE_BITMAP(vals, GPIOHANDLES_MAX);
unsigned int i;
int ret;
+ struct gpio_chip *gc;
- guard(srcu)(&lh->gdev->srcu);
-
- if (!rcu_access_pointer(lh->gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(lh->chip_rev, gc);
+ if (!gc)
return -ENODEV;
switch (cmd) {
@@ -279,7 +279,8 @@ static void linehandle_free(struct linehandle_state *lh)
if (lh->descs[i])
gpiod_free(lh->descs[i]);
kfree(lh->label);
- gpio_device_put(lh->gdev);
+ if (lh->chip_rev)
+ revocable_free(lh->chip_rev);
kfree(lh);
}
@@ -322,7 +323,10 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
lh = kzalloc(sizeof(*lh), GFP_KERNEL);
if (!lh)
return -ENOMEM;
- lh->gdev = gpio_device_get(gdev);
+
+ lh->chip_rev = revocable_alloc(gdev->chip_rp);
+ if (!lh->chip_rev)
+ return -ENOMEM;
if (handlereq.consumer_label[0] != '\0') {
/* label is only initialized if consumer_label is set */
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 15/23] gpiolib: cdev: Leverage revocable for line_fileops
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (13 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 14/23] gpiolib: cdev: Leverage revocable for linehandle_fileops Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 16/23] gpiolib: cdev: Leverage revocable for lineevent_fileops Tzung-Bi Shih
` (9 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Struct gpio_device now provides a revocable provider to the underlying
struct gpio_chip. Leverage revocable for line_fileops so that it doesn't
need to handle the synchronization by accessing the SRCU explicitly.
Also, it's unneeded to hold a reference count to the struct gpio_device
while the file is opening. The struct gpio_device
(i.e., (struct gpio_chip *)->gpiodev)) is valid as long as struct
gpio_chip is valid.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-cdev.c | 53 +++++++++++++++++++++++++------------
1 file changed, 36 insertions(+), 17 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index f7c6f1367235..f078d135a581 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -488,7 +488,7 @@ struct line {
/**
* struct linereq - contains the state of a userspace line request
- * @gdev: the GPIO device the line request pertains to
+ * @chip_rev: revocable consumer handle for the corresponding struct gpio_chip
* @label: consumer label used to tag GPIO descriptors
* @num_lines: the number of lines in the lines array
* @wait: wait queue that handles blocking reads of events
@@ -503,7 +503,7 @@ struct line {
* @lines: the lines held by this line request, with @num_lines elements.
*/
struct linereq {
- struct gpio_device *gdev;
+ struct revocable *chip_rev;
const char *label;
u32 num_lines;
wait_queue_head_t wait;
@@ -1437,10 +1437,10 @@ static long linereq_ioctl(struct file *file, unsigned int cmd,
{
struct linereq *lr = file->private_data;
void __user *ip = (void __user *)arg;
+ struct gpio_chip *gc;
- guard(srcu)(&lr->gdev->srcu);
-
- if (!rcu_access_pointer(lr->gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(lr->chip_rev, gc);
+ if (!gc)
return -ENODEV;
switch (cmd) {
@@ -1468,10 +1468,10 @@ static __poll_t linereq_poll(struct file *file,
{
struct linereq *lr = file->private_data;
__poll_t events = 0;
+ struct gpio_chip *gc;
- guard(srcu)(&lr->gdev->srcu);
-
- if (!rcu_access_pointer(lr->gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(lr->chip_rev, gc);
+ if (!gc)
return EPOLLHUP | EPOLLERR;
poll_wait(file, &lr->wait, wait);
@@ -1490,10 +1490,10 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
struct gpio_v2_line_event le;
ssize_t bytes_read = 0;
int ret;
+ struct gpio_chip *gc;
- guard(srcu)(&lr->gdev->srcu);
-
- if (!rcu_access_pointer(lr->gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(lr->chip_rev, gc);
+ if (!gc)
return -ENODEV;
if (count < sizeof(le))
@@ -1537,9 +1537,17 @@ static void linereq_free(struct linereq *lr)
{
unsigned int i;
- if (lr->device_unregistered_nb.notifier_call)
- blocking_notifier_chain_unregister(&lr->gdev->device_notifier,
- &lr->device_unregistered_nb);
+ if (lr->device_unregistered_nb.notifier_call) {
+ struct gpio_chip *gc;
+
+ REVOCABLE_TRY_ACCESS_WITH(lr->chip_rev, gc);
+ if (gc) {
+ struct gpio_device *gdev = gc->gpiodev;
+
+ blocking_notifier_chain_unregister(&gdev->device_notifier,
+ &lr->device_unregistered_nb);
+ }
+ }
for (i = 0; i < lr->num_lines; i++) {
if (lr->lines[i].desc) {
@@ -1549,7 +1557,8 @@ static void linereq_free(struct linereq *lr)
}
kfifo_free(&lr->events);
kfree(lr->label);
- gpio_device_put(lr->gdev);
+ if (lr->chip_rev)
+ revocable_free(lr->chip_rev);
kvfree(lr);
}
@@ -1565,9 +1574,15 @@ static int linereq_release(struct inode *inode, struct file *file)
static void linereq_show_fdinfo(struct seq_file *out, struct file *file)
{
struct linereq *lr = file->private_data;
- struct device *dev = &lr->gdev->dev;
+ struct gpio_chip *gc;
+ struct device *dev;
u16 i;
+ REVOCABLE_TRY_ACCESS_WITH(lr->chip_rev, gc);
+ if (!gc)
+ return;
+ dev = &gc->gpiodev->dev;
+
seq_printf(out, "gpio-chip:\t%s\n", dev_name(dev));
for (i = 0; i < lr->num_lines; i++)
@@ -1620,7 +1635,11 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
return -ENOMEM;
lr->num_lines = ulr.num_lines;
- lr->gdev = gpio_device_get(gdev);
+ lr->chip_rev = revocable_alloc(gdev->chip_rp);
+ if (!lr->chip_rev) {
+ ret = -ENOMEM;
+ goto out_free_linereq;
+ }
for (i = 0; i < ulr.num_lines; i++) {
lr->lines[i].req = lr;
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 16/23] gpiolib: cdev: Leverage revocable for lineevent_fileops
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (14 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 15/23] gpiolib: cdev: Leverage revocable for line_fileops Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 17/23] gpiolib: cdev: Leverage revocable for lineinfo_changed_notify Tzung-Bi Shih
` (8 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Struct gpio_device now provides a revocable provider to the underlying
struct gpio_chip. Leverage revocable for lineevent_fileops so that it
doesn't need to handle the synchronization by accessing the SRCU
explicitly.
Also, it's unneeded to hold a reference count to the struct gpio_device
while the file is opening. The struct gpio_device
(i.e., (struct gpio_chip *)->gpiodev)) is valid as long as struct
gpio_chip is valid.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-cdev.c | 46 ++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index f078d135a581..54150d718931 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1772,7 +1772,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
/**
* struct lineevent_state - contains the state of a userspace event
- * @gdev: the GPIO device the event pertains to
+ * @chip_rev: revocable consumer handle for the corresponding struct gpio_chip
* @label: consumer label used to tag descriptors
* @desc: the GPIO descriptor held by this event
* @eflags: the event flags this line was requested with
@@ -1785,7 +1785,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
* event
*/
struct lineevent_state {
- struct gpio_device *gdev;
+ struct revocable *chip_rev;
const char *label;
struct gpio_desc *desc;
u32 eflags;
@@ -1805,10 +1805,10 @@ static __poll_t lineevent_poll(struct file *file,
{
struct lineevent_state *le = file->private_data;
__poll_t events = 0;
+ struct gpio_chip *gc;
- guard(srcu)(&le->gdev->srcu);
-
- if (!rcu_access_pointer(le->gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(le->chip_rev, gc);
+ if (!gc)
return EPOLLHUP | EPOLLERR;
poll_wait(file, &le->wait, wait);
@@ -1843,10 +1843,10 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
ssize_t bytes_read = 0;
ssize_t ge_size;
int ret;
+ struct gpio_chip *gc;
- guard(srcu)(&le->gdev->srcu);
-
- if (!rcu_access_pointer(le->gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(le->chip_rev, gc);
+ if (!gc)
return -ENODEV;
/*
@@ -1901,15 +1901,24 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
static void lineevent_free(struct lineevent_state *le)
{
- if (le->device_unregistered_nb.notifier_call)
- blocking_notifier_chain_unregister(&le->gdev->device_notifier,
- &le->device_unregistered_nb);
+ if (le->device_unregistered_nb.notifier_call) {
+ struct gpio_chip *gc;
+
+ REVOCABLE_TRY_ACCESS_WITH(le->chip_rev, gc);
+ if (gc) {
+ struct gpio_device *gdev = gc->gpiodev;
+
+ blocking_notifier_chain_unregister(&gdev->device_notifier,
+ &le->device_unregistered_nb);
+ }
+ }
if (le->irq)
free_irq_label(free_irq(le->irq, le));
if (le->desc)
gpiod_free(le->desc);
kfree(le->label);
- gpio_device_put(le->gdev);
+ if (le->chip_rev)
+ revocable_free(le->chip_rev);
kfree(le);
}
@@ -1925,10 +1934,10 @@ static long lineevent_ioctl(struct file *file, unsigned int cmd,
struct lineevent_state *le = file->private_data;
void __user *ip = (void __user *)arg;
struct gpiohandle_data ghd;
+ struct gpio_chip *gc;
- guard(srcu)(&le->gdev->srcu);
-
- if (!rcu_access_pointer(le->gdev->chip))
+ REVOCABLE_TRY_ACCESS_WITH(le->chip_rev, gc);
+ if (!gc)
return -ENODEV;
/*
@@ -2081,7 +2090,12 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
le = kzalloc(sizeof(*le), GFP_KERNEL);
if (!le)
return -ENOMEM;
- le->gdev = gpio_device_get(gdev);
+
+ le->chip_rev = revocable_alloc(gdev->chip_rp);
+ if (!le->chip_rev) {
+ ret = -ENOMEM;
+ goto out_free_le;
+ }
if (eventreq.consumer_label[0] != '\0') {
/* label is only initialized if consumer_label is set */
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 17/23] gpiolib: cdev: Leverage revocable for lineinfo_changed_notify
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (15 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 16/23] gpiolib: cdev: Leverage revocable for lineevent_fileops Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 18/23] gpiolib: Leverage revocable for gpiolib_sops Tzung-Bi Shih
` (7 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Struct gpio_device now provides a revocable provider to the underlying
struct gpio_chip. Leverage revocable for lineinfo_changed_notify so
that it doesn't need to handle the synchronization by accessing the SRCU
explicitly.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-cdev.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 54150d718931..1a4dde56dc0c 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2522,6 +2522,7 @@ struct lineinfo_changed_ctx {
struct gpio_v2_line_info_changed chg;
struct gpio_device *gdev;
struct gpio_chardev_data *cdev;
+ struct revocable *chip_rev;
};
static void lineinfo_changed_func(struct work_struct *work)
@@ -2538,12 +2539,9 @@ static void lineinfo_changed_func(struct work_struct *work)
* Pin functions are in general much more static and while it's
* not 100% bullet-proof, it's good enough for most cases.
*/
- scoped_guard(srcu, &ctx->gdev->srcu) {
- gc = srcu_dereference(ctx->gdev->chip, &ctx->gdev->srcu);
- if (gc &&
- !pinctrl_gpio_can_use_line(gc, ctx->chg.info.offset))
- ctx->chg.info.flags |= GPIO_V2_LINE_FLAG_USED;
- }
+ REVOCABLE_TRY_ACCESS_WITH(ctx->chip_rev, gc);
+ if (gc && !pinctrl_gpio_can_use_line(gc, ctx->chg.info.offset))
+ ctx->chg.info.flags |= GPIO_V2_LINE_FLAG_USED;
}
ret = kfifo_in_spinlocked(&ctx->cdev->events, &ctx->chg, 1,
@@ -2553,6 +2551,7 @@ static void lineinfo_changed_func(struct work_struct *work)
else
pr_debug_ratelimited("lineinfo event FIFO is full - event dropped\n");
+ revocable_free(ctx->chip_rev);
gpio_device_put(ctx->gdev);
fput(ctx->cdev->fp);
kfree(ctx);
@@ -2599,11 +2598,19 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
/* Keep the GPIO device alive until we emit the event. */
ctx->gdev = gpio_device_get(desc->gdev);
ctx->cdev = cdev;
+ ctx->chip_rev = revocable_alloc(desc->gdev->chip_rp);
+ if (!ctx->chip_rev) {
+ pr_err("Failed to allocate memory for revocable handle\n");
+ goto err_put_device;
+ }
INIT_WORK(&ctx->work, lineinfo_changed_func);
queue_work(ctx->gdev->line_state_wq, &ctx->work);
return NOTIFY_OK;
+err_put_device:
+ gpio_device_put(desc->gdev);
+ kfree(ctx);
err_put_fp:
fput(fp);
return NOTIFY_DONE;
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 18/23] gpiolib: Leverage revocable for gpiolib_sops
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (16 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 17/23] gpiolib: cdev: Leverage revocable for lineinfo_changed_notify Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 19/23] revocable: Support to define revocable consumer handle on stack Tzung-Bi Shih
` (6 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Struct gpio_device now provides a revocable provider to the underlying
struct gpio_chip. Leverage revocable for gpiolib_sops so that it
doesn't need to handle the synchronization by accessing the SRCU
explicitly.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 6226dc738281..cd18ff42b610 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -5376,6 +5376,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *gc)
struct gpiolib_seq_priv {
bool newline;
int idx;
+ struct revocable *chip_rev;
};
static void *gpiolib_seq_start(struct seq_file *s, loff_t *pos)
@@ -5397,8 +5398,12 @@ static void *gpiolib_seq_start(struct seq_file *s, loff_t *pos)
list_for_each_entry_srcu(gdev, &gpio_devices, list,
srcu_read_lock_held(&gpio_devices_srcu)) {
- if (index-- == 0)
+ if (index-- == 0) {
+ priv->chip_rev = revocable_alloc(gdev->chip_rp);
+ if (!priv->chip_rev)
+ return NULL;
return gdev;
+ }
}
return NULL;
@@ -5425,6 +5430,8 @@ static void gpiolib_seq_stop(struct seq_file *s, void *v)
if (!priv)
return;
+ if (priv->chip_rev)
+ revocable_free(priv->chip_rev);
srcu_read_unlock(&gpio_devices_srcu, priv->idx);
kfree(priv);
}
@@ -5439,9 +5446,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
if (priv->newline)
seq_putc(s, '\n');
- guard(srcu)(&gdev->srcu);
-
- gc = srcu_dereference(gdev->chip, &gdev->srcu);
+ REVOCABLE_TRY_ACCESS_WITH(priv->chip_rev, gc);
if (!gc) {
seq_printf(s, "%s: (dangling chip)\n", dev_name(&gdev->dev));
return 0;
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 19/23] revocable: Support to define revocable consumer handle on stack
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (17 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 18/23] gpiolib: Leverage revocable for gpiolib_sops Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 20/23] revocable: Add Kunit test case for DEFINE_REVOCABLE() Tzung-Bi Shih
` (5 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Support a way to define a revocable consumer handle on stack. Under
some circumstances, the user wouldn't like to use dynamic memory
allocation for consumer handles.
This makes the struct revocable no longer opaque.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
.../driver-api/driver-model/revocable.rst | 5 +-
drivers/base/revocable.c | 60 +++++++++++++------
include/linux/revocable.h | 30 +++++++++-
3 files changed, 74 insertions(+), 21 deletions(-)
diff --git a/Documentation/driver-api/driver-model/revocable.rst b/Documentation/driver-api/driver-model/revocable.rst
index 22a442cc8d7f..fff081dbd296 100644
--- a/Documentation/driver-api/driver-model/revocable.rst
+++ b/Documentation/driver-api/driver-model/revocable.rst
@@ -84,7 +84,7 @@ For Resource Providers
For Resource Consumers
----------------------
-.. kernel-doc:: drivers/base/revocable.c
+.. kernel-doc:: include/linux/revocable.h
:identifiers: revocable
.. kernel-doc:: drivers/base/revocable.c
@@ -93,6 +93,9 @@ For Resource Consumers
.. kernel-doc:: drivers/base/revocable.c
:identifiers: revocable_free
+.. kernel-doc:: include/linux/revocable.h
+ :identifiers: DEFINE_REVOCABLE
+
.. kernel-doc:: drivers/base/revocable.c
:identifiers: revocable_try_access
diff --git a/drivers/base/revocable.c b/drivers/base/revocable.c
index f6cece275aac..93c925252665 100644
--- a/drivers/base/revocable.c
+++ b/drivers/base/revocable.c
@@ -71,16 +71,6 @@ struct revocable_provider {
struct kref kref;
};
-/**
- * struct revocable - A handle for resource consumer.
- * @rp: The pointer of resource provider.
- * @idx: The index for the RCU critical section.
- */
-struct revocable {
- struct revocable_provider *rp;
- int idx;
-};
-
/**
* revocable_provider_alloc() - Allocate struct revocable_provider.
* @res: The pointer of resource.
@@ -170,11 +160,47 @@ struct revocable_provider *devm_revocable_provider_alloc(struct device *dev,
EXPORT_SYMBOL_GPL(devm_revocable_provider_alloc);
/**
- * revocable_alloc() - Allocate struct revocable.
+ * revocable_init() - Initialize struct revocable.
+ * @rev: The pointer of struct revocable.
* @rp: The pointer of resource provider.
*
* This holds a refcount to the resource provider.
*
+ * Don't call this function directly. Use revocable_alloc() or
+ * DEFINE_REVOCABLE().
+ */
+void revocable_init(struct revocable *rev, struct revocable_provider *rp)
+{
+ rev->rp = rp;
+ kref_get(&rp->kref);
+}
+EXPORT_SYMBOL_GPL(revocable_init);
+
+/**
+ * revocable_deinit() - Deinitialize struct revocable.
+ * @rev: The pointer of struct revocable.
+ *
+ * This drops a refcount to the resource provider. If it is the final
+ * reference, revocable_provider_release() will be called to free the struct.
+ *
+ * Don't call this function directly. revocable_free() or DEFINE_REVOCABLE()
+ * should help to do so.
+ */
+void revocable_deinit(struct revocable *rev)
+{
+ struct revocable_provider *rp = rev->rp;
+
+ kref_put(&rp->kref, revocable_provider_release);
+}
+EXPORT_SYMBOL_GPL(revocable_deinit);
+
+/**
+ * revocable_alloc() - Allocate struct revocable.
+ * @rp: The pointer of resource provider.
+ *
+ * Allocate a struct revocable and call revocable_init() to holds a refcount
+ * to the resource provider.
+ *
* Return: The pointer of struct revocable. NULL on errors.
*/
struct revocable *revocable_alloc(struct revocable_provider *rp)
@@ -185,9 +211,7 @@ struct revocable *revocable_alloc(struct revocable_provider *rp)
if (!rev)
return NULL;
- rev->rp = rp;
- kref_get(&rp->kref);
-
+ revocable_init(rev, rp);
return rev;
}
EXPORT_SYMBOL_GPL(revocable_alloc);
@@ -196,14 +220,12 @@ EXPORT_SYMBOL_GPL(revocable_alloc);
* revocable_free() - Free struct revocable.
* @rev: The pointer of struct revocable.
*
- * This drops a refcount to the resource provider. If it is the final
- * reference, revocable_provider_release() will be called to free the struct.
+ * Call revocable_deinit() to drop a refcount to the resource provider and
+ * free the struct revocable.
*/
void revocable_free(struct revocable *rev)
{
- struct revocable_provider *rp = rev->rp;
-
- kref_put(&rp->kref, revocable_provider_release);
+ revocable_deinit(rev);
kfree(rev);
}
EXPORT_SYMBOL_GPL(revocable_free);
diff --git a/include/linux/revocable.h b/include/linux/revocable.h
index 659ba01c58db..89bb1a5c74e4 100644
--- a/include/linux/revocable.h
+++ b/include/linux/revocable.h
@@ -10,19 +10,47 @@
#include <linux/cleanup.h>
struct device;
-struct revocable;
struct revocable_provider;
+/**
+ * struct revocable - A handle for resource consumer.
+ * @rp: The pointer of resource provider.
+ * @idx: The index for the RCU critical section.
+ */
+struct revocable {
+ struct revocable_provider *rp;
+ int idx;
+};
+
struct revocable_provider *revocable_provider_alloc(void *res);
void revocable_provider_revoke(struct revocable_provider *rp);
struct revocable_provider *devm_revocable_provider_alloc(struct device *dev,
void *res);
+void revocable_init(struct revocable *rev, struct revocable_provider *rp);
+void revocable_deinit(struct revocable *rev);
struct revocable *revocable_alloc(struct revocable_provider *rp);
void revocable_free(struct revocable *rev);
void *revocable_try_access(struct revocable *rev) __acquires(&rev->rp->srcu);
void revocable_withdraw_access(struct revocable *rev) __releases(&rev->rp->srcu);
+DEFINE_FREE(define_rev, struct revocable *, revocable_deinit(_T))
+
+#define _DEFINE_REVOCABLE(_rev, _name, _rp) \
+ struct revocable _name; \
+ struct revocable *_rev __free(define_rev) = &_name; \
+ revocable_init(_rev, _rp)
+
+/**
+ * DEFINE_REVOCABLE() - A helper for defining a revocable consumer on stack
+ * @_rev: The variable name to ``struct revocable *``.
+ * @_rp: The provider's ``struct revocable_provider *`` handle.
+ *
+ * The macro declares and defines a revocable consumer handle on stack.
+ */
+#define DEFINE_REVOCABLE(_rev, _rp) \
+ _DEFINE_REVOCABLE(_rev, __UNIQUE_ID(name), _rp)
+
DEFINE_FREE(access_rev, struct revocable *, if (_T) revocable_withdraw_access(_T))
/**
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 20/23] revocable: Add Kunit test case for DEFINE_REVOCABLE()
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (18 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 19/23] revocable: Support to define revocable consumer handle on stack Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 21/23] selftests: revocable: Add " Tzung-Bi Shih
` (4 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Add Kunit test case to verify DEFINE_REVOCABLE() can successfully access
the resource and drop the reference count.
A way to run the test:
$ ./tools/testing/kunit/kunit.py run \
--kconfig_add CONFIG_REVOCABLE_KUNIT_TEST=y \
revocable_test
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/base/revocable_test.c | 54 +++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/drivers/base/revocable_test.c b/drivers/base/revocable_test.c
index 28d46ce1ba0c..e0efffe6e04f 100644
--- a/drivers/base/revocable_test.c
+++ b/drivers/base/revocable_test.c
@@ -14,10 +14,15 @@
*
* - Try Access Macro: Same as "Revocation" but uses the
* REVOCABLE_TRY_ACCESS_WITH() and REVOCABLE_TRY_ACCESS_SCOPED().
+ *
+ * - Define Macro: Verifies DEFINE_REVOCABLE() can successfully access the
+ * resource and drop the reference count.
*/
#include <kunit/test.h>
+#include <linux/kref.h>
#include <linux/revocable.h>
+#include <linux/srcu.h>
static void revocable_test_basic(struct kunit *test)
{
@@ -123,11 +128,60 @@ static void revocable_test_try_access_macro2(struct kunit *test)
revocable_free(rev);
}
+static void revocable_test_define_macro(struct kunit *test)
+{
+ /* To access the opaque struct */
+ struct {
+ struct srcu_struct srcu;
+ void *res;
+ struct kref kref;
+ } *_rp;
+ struct revocable_provider *rp;
+ void *real_res = (void *)0x12345678, *res;
+
+ rp = revocable_provider_alloc(real_res);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rp);
+
+ _rp = (void *)rp;
+ KUNIT_EXPECT_EQ(test, kref_read(&_rp->kref), 1);
+
+ {
+ DEFINE_REVOCABLE(rev, rp);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rev);
+ KUNIT_EXPECT_EQ(test, kref_read(&_rp->kref), 2);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, res);
+ KUNIT_EXPECT_PTR_EQ(test, res, real_res);
+ }
+ KUNIT_EXPECT_EQ(test, kref_read(&_rp->kref), 1);
+
+ {
+ bool accessed = false;
+
+ DEFINE_REVOCABLE(rev, rp);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rev);
+ KUNIT_EXPECT_EQ(test, kref_read(&_rp->kref), 2);
+
+ REVOCABLE_TRY_ACCESS_SCOPED(rev, res) {
+ KUNIT_EXPECT_PTR_EQ(test, res, real_res);
+ accessed = true;
+ }
+ KUNIT_EXPECT_TRUE(test, accessed);
+
+ revocable_provider_revoke(rp);
+ KUNIT_EXPECT_EQ(test, kref_read(&_rp->kref), 1);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, res);
+ KUNIT_EXPECT_PTR_EQ(test, res, NULL);
+ }
+}
+
static struct kunit_case revocable_test_cases[] = {
KUNIT_CASE(revocable_test_basic),
KUNIT_CASE(revocable_test_revocation),
KUNIT_CASE(revocable_test_try_access_macro),
KUNIT_CASE(revocable_test_try_access_macro2),
+ KUNIT_CASE(revocable_test_define_macro),
{}
};
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 21/23] selftests: revocable: Add test case for DEFINE_REVOCABLE()
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (19 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 20/23] revocable: Add Kunit test case for DEFINE_REVOCABLE() Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 8:10 ` [PATCH 22/23] gpiolib: Leverage revocable for other independent lifecycle instances Tzung-Bi Shih
` (3 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
Add selftest case to verify DEFINE_REVOCABLE() can successfully access
the resource.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
.../drivers/base/revocable/revocable_test.c | 12 ++++++
.../revocable/test_modules/revocable_test.c | 37 +++++++++++++++----
2 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/drivers/base/revocable/revocable_test.c b/tools/testing/selftests/drivers/base/revocable/revocable_test.c
index f024164e9273..723e5ea9ec84 100644
--- a/tools/testing/selftests/drivers/base/revocable/revocable_test.c
+++ b/tools/testing/selftests/drivers/base/revocable/revocable_test.c
@@ -14,6 +14,9 @@
*
* - Try Access Macro: Same as "Revocation" but uses the
* REVOCABLE_TRY_ACCESS_WITH() and REVOCABLE_TRY_ACCESS_SCOPED().
+ *
+ * - Define Macro: Verifies DEFINE_REVOCABLE() can successfully access the
+ * resource.
*/
#include <fcntl.h>
@@ -26,6 +29,7 @@
#define TEST_DATA "12345678"
#define TEST_MAGIC_OFFSET 0x1234
#define TEST_MAGIC_OFFSET2 0x5678
+#define TEST_MAGIC_OFFSET3 0x9abc
FIXTURE(revocable_fixture) {
int pfd;
@@ -133,4 +137,12 @@ TEST_F(revocable_fixture, try_access_macro2) {
EXPECT_STREQ("(null)", data);
}
+TEST_F(revocable_fixture, define_macro) {
+ char data[16];
+
+ READ_TEST_DATA(self->cfd, TEST_MAGIC_OFFSET3, data,
+ "failed to read test data");
+ EXPECT_STREQ(TEST_DATA, data);
+}
+
TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c b/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c
index 1b0692eb75f3..d3bd0ce3a645 100644
--- a/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c
+++ b/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c
@@ -13,6 +13,7 @@
#define TEST_CMD_RESOURCE_GONE "resource_gone"
#define TEST_MAGIC_OFFSET 0x1234
#define TEST_MAGIC_OFFSET2 0x5678
+#define TEST_MAGIC_OFFSET3 0x9abc
static struct dentry *debugfs_dir;
@@ -22,15 +23,27 @@ struct revocable_test_provider_priv {
char res[16];
};
+struct revocable_test_consumer_priv {
+ struct revocable_provider *rp;
+ struct revocable *rev;
+};
+
static int revocable_test_consumer_open(struct inode *inode, struct file *filp)
{
- struct revocable *rev;
struct revocable_provider *rp = inode->i_private;
+ struct revocable_test_consumer_priv *priv;
- rev = revocable_alloc(rp);
- if (!rev)
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
return -ENOMEM;
- filp->private_data = rev;
+
+ priv->rp = rp;
+ priv->rev = revocable_alloc(rp);
+ if (!priv->rev) {
+ kfree(priv);
+ return -ENOMEM;
+ }
+ filp->private_data = priv;
return 0;
}
@@ -38,9 +51,10 @@ static int revocable_test_consumer_open(struct inode *inode, struct file *filp)
static int revocable_test_consumer_release(struct inode *inode,
struct file *filp)
{
- struct revocable *rev = filp->private_data;
+ struct revocable_test_consumer_priv *priv = filp->private_data;
- revocable_free(rev);
+ revocable_free(priv->rev);
+ kfree(priv);
return 0;
}
@@ -51,7 +65,8 @@ static ssize_t revocable_test_consumer_read(struct file *filp,
char *res;
char data[16];
size_t len;
- struct revocable *rev = filp->private_data;
+ struct revocable_test_consumer_priv *priv = filp->private_data;
+ struct revocable *rev = priv->rev;
switch (*offset) {
case 0:
@@ -69,6 +84,14 @@ static ssize_t revocable_test_consumer_read(struct file *filp,
REVOCABLE_TRY_ACCESS_SCOPED(rev, res)
snprintf(data, sizeof(data), "%s", res ?: "(null)");
break;
+ case TEST_MAGIC_OFFSET3:
+ {
+ DEFINE_REVOCABLE(rev_on_stack, priv->rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev_on_stack, res);
+ snprintf(data, sizeof(data), "%s", res ?: "(null)");
+ }
+ break;
default:
return 0;
}
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH 22/23] gpiolib: Leverage revocable for other independent lifecycle instances
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (20 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 21/23] selftests: revocable: Add " Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-24 16:52 ` Johan Hovold
2026-01-16 8:10 ` [PATCH 23/23] gpiolib: Remove unused `chip` and `srcu` in struct gpio_device Tzung-Bi Shih
` (2 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
There are independent lifecycle instances (e.g., other drivers) can save
a raw pointer to the struct gpio_device (e.g., via gpio_device_find())
or struct gpio_desc (e.g., via gpio_to_desc()). In some operations,
they have to access the underlying struct gpio_chip.
Leverage revocable for them so that they don't need to handle the
synchronization by accessing the SRCU explicitly.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib-cdev.c | 10 +-
drivers/gpio/gpiolib-sysfs.c | 35 +++--
drivers/gpio/gpiolib.c | 241 ++++++++++++++++++++---------------
drivers/gpio/gpiolib.h | 21 ---
4 files changed, 163 insertions(+), 144 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 1a4dde56dc0c..ecb1472b5c8f 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2252,9 +2252,11 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
u32 debounce_period_us;
unsigned long dflags;
const char *label;
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return;
memset(info, 0, sizeof(*info));
@@ -2288,10 +2290,10 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
test_bit(GPIOD_FLAG_IS_HOGGED, &dflags) ||
test_bit(GPIOD_FLAG_EXPORT, &dflags) ||
test_bit(GPIOD_FLAG_SYSFS, &dflags) ||
- !gpiochip_line_is_valid(guard.gc, info->offset)) {
+ !gpiochip_line_is_valid(gc, info->offset)) {
info->flags |= GPIO_V2_LINE_FLAG_USED;
} else if (!atomic) {
- if (!pinctrl_gpio_can_use_line(guard.gc, info->offset))
+ if (!pinctrl_gpio_can_use_line(gc, info->offset))
info->flags |= GPIO_V2_LINE_FLAG_USED;
}
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index a4427a5cfa85..bc8d4af73cc3 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -215,9 +215,11 @@ static int gpio_sysfs_request_irq(struct gpiod_data *data, unsigned char flags)
struct gpio_desc *desc = data->desc;
unsigned long irq_flags;
int ret;
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
data->irq = gpiod_to_irq(desc);
@@ -244,7 +246,7 @@ static int gpio_sysfs_request_irq(struct gpiod_data *data, unsigned char flags)
* Remove this redundant call (along with the corresponding unlock)
* when those drivers have been fixed.
*/
- ret = gpiochip_lock_as_irq(guard.gc, gpiod_hwgpio(desc));
+ ret = gpiochip_lock_as_irq(gc, gpiod_hwgpio(desc));
if (ret < 0)
goto err_clr_bits;
@@ -258,7 +260,7 @@ static int gpio_sysfs_request_irq(struct gpiod_data *data, unsigned char flags)
return 0;
err_unlock:
- gpiochip_unlock_as_irq(guard.gc, gpiod_hwgpio(desc));
+ gpiochip_unlock_as_irq(gc, gpiod_hwgpio(desc));
err_clr_bits:
clear_bit(GPIOD_FLAG_EDGE_RISING, &desc->flags);
clear_bit(GPIOD_FLAG_EDGE_FALLING, &desc->flags);
@@ -273,14 +275,16 @@ static int gpio_sysfs_request_irq(struct gpiod_data *data, unsigned char flags)
static void gpio_sysfs_free_irq(struct gpiod_data *data)
{
struct gpio_desc *desc = data->desc;
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return;
data->irq_flags = 0;
free_irq(data->irq, data);
- gpiochip_unlock_as_irq(guard.gc, gpiod_hwgpio(desc));
+ gpiochip_unlock_as_irq(gc, gpiod_hwgpio(desc));
clear_bit(GPIOD_FLAG_EDGE_RISING, &desc->flags);
clear_bit(GPIOD_FLAG_EDGE_FALLING, &desc->flags);
}
@@ -473,13 +477,15 @@ static DEVICE_ATTR_RO(ngpio);
static int export_gpio_desc(struct gpio_desc *desc)
{
int offset, ret;
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
offset = gpiod_hwgpio(desc);
- if (!gpiochip_line_is_valid(guard.gc, offset)) {
+ if (!gpiochip_line_is_valid(gc, offset)) {
pr_debug_ratelimited("%s: GPIO %d masked\n", __func__,
gpiod_hwgpio(desc));
return -EINVAL;
@@ -732,6 +738,7 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
struct gpio_device *gdev;
struct attribute **attrs;
int status;
+ struct gpio_chip *gc;
/* can't export until sysfs is available ... */
if (!class_is_registered(&gpio_class)) {
@@ -744,8 +751,10 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
return -EINVAL;
}
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
if (test_and_set_bit(GPIOD_FLAG_EXPORT, &desc->flags))
@@ -769,7 +778,7 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
desc_data->desc = desc;
mutex_init(&desc_data->mutex);
- if (guard.gc->direction_input && guard.gc->direction_output)
+ if (gc->direction_input && gc->direction_output)
desc_data->direction_can_change = direction_may_change;
else
desc_data->direction_can_change = false;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cd18ff42b610..44915c8b6131 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -338,7 +338,11 @@ EXPORT_SYMBOL(gpio_device_get_label);
*/
struct gpio_chip *gpio_device_get_chip(struct gpio_device *gdev)
{
- return rcu_dereference_check(gdev->chip, 1);
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ return gc;
}
EXPORT_SYMBOL_GPL(gpio_device_get_chip);
@@ -449,13 +453,16 @@ int gpiod_get_direction(struct gpio_desc *desc)
unsigned long flags;
unsigned int offset;
int ret;
+ struct gpio_chip *gc;
ret = validate_desc(desc, __func__);
if (ret <= 0)
return -EINVAL;
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
offset = gpiod_hwgpio(desc);
@@ -469,7 +476,7 @@ int gpiod_get_direction(struct gpio_desc *desc)
test_bit(GPIOD_FLAG_IS_OUT, &flags))
return 0;
- ret = gpiochip_get_direction(guard.gc, offset);
+ ret = gpiochip_get_direction(gc, offset);
if (ret < 0)
return ret;
@@ -557,9 +564,9 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
list_for_each_entry_srcu(gdev, &gpio_devices, list,
srcu_read_lock_held(&gpio_devices_srcu)) {
- guard(srcu)(&gdev->srcu);
+ DEFINE_REVOCABLE(rev, gdev->chip_rp);
- gc = srcu_dereference(gdev->chip, &gdev->srcu);
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
if (!gc)
continue;
@@ -971,9 +978,9 @@ static void gpiochip_setup_devs(void)
list_for_each_entry_srcu(gdev, &gpio_devices, list,
srcu_read_lock_held(&gpio_devices_srcu)) {
- guard(srcu)(&gdev->srcu);
+ DEFINE_REVOCABLE(rev, gdev->chip_rp);
- gc = srcu_dereference(gdev->chip, &gdev->srcu);
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
if (!gc) {
dev_err(&gdev->dev, "Underlying GPIO chip is gone\n");
continue;
@@ -1386,11 +1393,13 @@ struct gpio_device *gpio_device_find(const void *data,
if (!device_is_registered(&gdev->dev))
continue;
- guard(srcu)(&gdev->srcu);
+ DEFINE_REVOCABLE(rev, gdev->chip_rp);
- gc = srcu_dereference(gdev->chip, &gdev->srcu);
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
+ continue;
- if (gc && match(gc, data))
+ if (match(gc, data))
return gpio_device_get(gdev);
}
@@ -2482,31 +2491,33 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
{
unsigned int offset;
int ret;
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
if (test_and_set_bit(GPIOD_FLAG_REQUESTED, &desc->flags))
return -EBUSY;
offset = gpiod_hwgpio(desc);
- if (!gpiochip_line_is_valid(guard.gc, offset))
+ if (!gpiochip_line_is_valid(gc, offset))
return -EINVAL;
/* NOTE: gpio_request() can be called in early boot,
* before IRQs are enabled, for non-sleeping (SOC) GPIOs.
*/
- if (guard.gc->request) {
- ret = guard.gc->request(guard.gc, offset);
+ if (gc->request) {
+ ret = gc->request(gc, offset);
if (ret > 0)
ret = -EBADE;
if (ret)
goto out_clear_bit;
}
- if (guard.gc->get_direction)
+ if (gc->get_direction)
gpiod_get_direction(desc);
ret = desc_set_label(desc, label ? : "?");
@@ -2543,16 +2554,21 @@ int gpiod_request(struct gpio_desc *desc, const char *label)
static void gpiod_free_commit(struct gpio_desc *desc)
{
unsigned long flags;
+ struct gpio_chip *gc;
might_sleep();
- CLASS(gpio_chip_guard, guard)(desc);
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
+ return;
flags = READ_ONCE(desc->flags);
- if (guard.gc && test_bit(GPIOD_FLAG_REQUESTED, &flags)) {
- if (guard.gc->free)
- guard.gc->free(guard.gc, gpiod_hwgpio(desc));
+ if (gc && test_bit(GPIOD_FLAG_REQUESTED, &flags)) {
+ if (gc->free)
+ gc->free(gc, gpiod_hwgpio(desc));
clear_bit(GPIOD_FLAG_ACTIVE_LOW, &flags);
clear_bit(GPIOD_FLAG_REQUESTED, &flags);
@@ -2704,15 +2720,17 @@ EXPORT_SYMBOL_GPL(gpiochip_free_own_desc);
int gpio_do_set_config(struct gpio_desc *desc, unsigned long config)
{
int ret;
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
- if (!guard.gc->set_config)
+ if (!gc->set_config)
return -ENOTSUPP;
- ret = guard.gc->set_config(guard.gc, gpiod_hwgpio(desc), config);
+ ret = gc->set_config(gc, gpiod_hwgpio(desc), config);
if (ret > 0)
ret = -EBADE;
@@ -2881,9 +2899,11 @@ EXPORT_SYMBOL_GPL(gpiod_direction_input);
int gpiod_direction_input_nonotify(struct gpio_desc *desc)
{
int ret = 0, dir;
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
/*
@@ -2891,7 +2911,7 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
* the chip is output-only, but you can't specify .direction_input()
* and not support the .get() operation, that doesn't make sense.
*/
- if (!guard.gc->get && guard.gc->direction_input) {
+ if (!gc->get && gc->direction_input) {
gpiod_warn(desc,
"%s: missing get() but have direction_input()\n",
__func__);
@@ -2904,11 +2924,10 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
* direction (if .get_direction() is supported) else we silently
* assume we are in input mode after this.
*/
- if (guard.gc->direction_input) {
- ret = gpiochip_direction_input(guard.gc,
- gpiod_hwgpio(desc));
- } else if (guard.gc->get_direction) {
- dir = gpiochip_get_direction(guard.gc, gpiod_hwgpio(desc));
+ if (gc->direction_input) {
+ ret = gpiochip_direction_input(gc, gpiod_hwgpio(desc));
+ } else if (gc->get_direction) {
+ dir = gpiochip_get_direction(gc, gpiod_hwgpio(desc));
if (dir < 0)
return dir;
@@ -2948,9 +2967,11 @@ static int gpiochip_set(struct gpio_chip *gc, unsigned int offset, int value)
static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
{
int val = !!value, ret = 0, dir;
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
/*
@@ -2958,21 +2979,19 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
* output-only, but if there is then not even a .set() operation it
* is pretty tricky to drive the output line.
*/
- if (!guard.gc->set && !guard.gc->direction_output) {
+ if (!gc->set && !gc->direction_output) {
gpiod_warn(desc,
"%s: missing set() and direction_output() operations\n",
__func__);
return -EIO;
}
- if (guard.gc->direction_output) {
- ret = gpiochip_direction_output(guard.gc,
- gpiod_hwgpio(desc), val);
+ if (gc->direction_output) {
+ ret = gpiochip_direction_output(gc, gpiod_hwgpio(desc), val);
} else {
/* Check that we are in output mode if we can */
- if (guard.gc->get_direction) {
- dir = gpiochip_get_direction(guard.gc,
- gpiod_hwgpio(desc));
+ if (gc->get_direction) {
+ dir = gpiochip_get_direction(gc, gpiod_hwgpio(desc));
if (dir < 0)
return dir;
@@ -2987,7 +3006,7 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
* If we can't actively set the direction, we are some
* output-only chip, so just drive the output as desired.
*/
- ret = gpiochip_set(guard.gc, gpiod_hwgpio(desc), val);
+ ret = gpiochip_set(gc, gpiod_hwgpio(desc), val);
if (ret)
return ret;
}
@@ -3125,20 +3144,22 @@ int gpiod_direction_output_nonotify(struct gpio_desc *desc, int value)
int gpiod_enable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags)
{
int ret;
+ struct gpio_chip *gc;
VALIDATE_DESC(desc);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
- if (!guard.gc->en_hw_timestamp) {
+ if (!gc->en_hw_timestamp) {
gpiod_warn(desc, "%s: hw ts not supported\n", __func__);
return -ENOTSUPP;
}
- ret = guard.gc->en_hw_timestamp(guard.gc,
- gpiod_hwgpio(desc), flags);
+ ret = gc->en_hw_timestamp(gc, gpiod_hwgpio(desc), flags);
if (ret)
gpiod_warn(desc, "%s: hw ts request failed\n", __func__);
@@ -3158,20 +3179,22 @@ EXPORT_SYMBOL_GPL(gpiod_enable_hw_timestamp_ns);
int gpiod_disable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags)
{
int ret;
+ struct gpio_chip *gc;
VALIDATE_DESC(desc);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
- if (!guard.gc->dis_hw_timestamp) {
+ if (!gc->dis_hw_timestamp) {
gpiod_warn(desc, "%s: hw ts not supported\n", __func__);
return -ENOTSUPP;
}
- ret = guard.gc->dis_hw_timestamp(guard.gc, gpiod_hwgpio(desc),
- flags);
+ ret = gc->dis_hw_timestamp(gc, gpiod_hwgpio(desc), flags);
if (ret)
gpiod_warn(desc, "%s: hw ts release failed\n", __func__);
@@ -3328,16 +3351,11 @@ static int gpio_chip_get_value(struct gpio_chip *gc, const struct gpio_desc *des
static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
{
- struct gpio_device *gdev;
struct gpio_chip *gc;
int value;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- /* FIXME Unable to use gpio_chip_guard due to const desc. */
- gdev = desc->gdev;
-
- guard(srcu)(&gdev->srcu);
-
- gc = srcu_dereference(gdev->chip, &gdev->srcu);
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
if (!gc)
return -ENODEV;
@@ -3378,9 +3396,11 @@ static int gpio_chip_get_multiple(struct gpio_chip *gc,
/* The 'other' chip must be protected with its GPIO device's SRCU. */
static bool gpio_device_chip_cmp(struct gpio_device *gdev, struct gpio_chip *gc)
{
- guard(srcu)(&gdev->srcu);
+ struct gpio_chip *chip;
+ DEFINE_REVOCABLE(rev, gdev->chip_rp);
- return gc == srcu_dereference(gdev->chip, &gdev->srcu);
+ REVOCABLE_TRY_ACCESS_WITH(rev, chip);
+ return chip ? chip == gc : false;
}
int gpiod_get_array_value_complex(bool raw, bool can_sleep,
@@ -3403,9 +3423,9 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
if (!can_sleep)
WARN_ON(array_info->gdev->can_sleep);
- guard(srcu)(&array_info->gdev->srcu);
- gc = srcu_dereference(array_info->gdev->chip,
- &array_info->gdev->srcu);
+ DEFINE_REVOCABLE(rev, array_info->gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
if (!gc)
return -ENODEV;
@@ -3430,32 +3450,33 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
DECLARE_BITMAP(fastpath_bits, FASTPATH_NGPIO);
unsigned long *mask, *bits;
int first, j;
+ DEFINE_REVOCABLE(rev, desc_array[i]->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc_array[i]);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
- if (likely(guard.gc->ngpio <= FASTPATH_NGPIO)) {
+ if (likely(gc->ngpio <= FASTPATH_NGPIO)) {
mask = fastpath_mask;
bits = fastpath_bits;
} else {
gfp_t flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
- mask = bitmap_alloc(guard.gc->ngpio, flags);
+ mask = bitmap_alloc(gc->ngpio, flags);
if (!mask)
return -ENOMEM;
- bits = bitmap_alloc(guard.gc->ngpio, flags);
+ bits = bitmap_alloc(gc->ngpio, flags);
if (!bits) {
bitmap_free(mask);
return -ENOMEM;
}
}
- bitmap_zero(mask, guard.gc->ngpio);
+ bitmap_zero(mask, gc->ngpio);
if (!can_sleep)
- WARN_ON(guard.gc->can_sleep);
+ WARN_ON(gc->can_sleep);
/* collect all inputs belonging to the same chip */
first = i;
@@ -3470,9 +3491,9 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
i = find_next_zero_bit(array_info->get_mask,
array_size, i);
} while ((i < array_size) &&
- gpio_device_chip_cmp(desc_array[i]->gdev, guard.gc));
+ gpio_device_chip_cmp(desc_array[i]->gdev, gc));
- ret = gpio_chip_get_multiple(guard.gc, mask, bits);
+ ret = gpio_chip_get_multiple(gc, mask, bits);
if (ret) {
if (mask != fastpath_mask)
bitmap_free(mask);
@@ -3621,15 +3642,17 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_value);
static int gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
{
int ret = 0, offset = gpiod_hwgpio(desc);
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
if (value) {
- ret = gpiochip_direction_input(guard.gc, offset);
+ ret = gpiochip_direction_input(gc, offset);
} else {
- ret = gpiochip_direction_output(guard.gc, offset, 0);
+ ret = gpiochip_direction_output(gc, offset, 0);
if (!ret)
set_bit(GPIOD_FLAG_IS_OUT, &desc->flags);
}
@@ -3650,17 +3673,19 @@ static int gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
static int gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value)
{
int ret = 0, offset = gpiod_hwgpio(desc);
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
if (value) {
- ret = gpiochip_direction_output(guard.gc, offset, 1);
+ ret = gpiochip_direction_output(gc, offset, 1);
if (!ret)
set_bit(GPIOD_FLAG_IS_OUT, &desc->flags);
} else {
- ret = gpiochip_direction_input(guard.gc, offset);
+ ret = gpiochip_direction_input(gc, offset);
}
trace_gpio_direction(desc_to_gpio(desc), !value, ret);
if (ret < 0)
@@ -3673,15 +3698,19 @@ static int gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value)
static int gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)
{
+ struct gpio_chip *gc;
+
if (unlikely(!test_bit(GPIOD_FLAG_IS_OUT, &desc->flags)))
return -EPERM;
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
trace_gpio_value(desc_to_gpio(desc), 0, value);
- return gpiochip_set(guard.gc, gpiod_hwgpio(desc), value);
+ return gpiochip_set(gc, gpiod_hwgpio(desc), value);
}
/*
@@ -3748,9 +3777,9 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
return -EPERM;
}
- guard(srcu)(&array_info->gdev->srcu);
- gc = srcu_dereference(array_info->gdev->chip,
- &array_info->gdev->srcu);
+ DEFINE_REVOCABLE(rev, array_info->gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
if (!gc)
return -ENODEV;
@@ -3775,32 +3804,33 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
DECLARE_BITMAP(fastpath_bits, FASTPATH_NGPIO);
unsigned long *mask, *bits;
int count = 0;
+ DEFINE_REVOCABLE(rev, desc_array[i]->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc_array[i]);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
- if (likely(guard.gc->ngpio <= FASTPATH_NGPIO)) {
+ if (likely(gc->ngpio <= FASTPATH_NGPIO)) {
mask = fastpath_mask;
bits = fastpath_bits;
} else {
gfp_t flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
- mask = bitmap_alloc(guard.gc->ngpio, flags);
+ mask = bitmap_alloc(gc->ngpio, flags);
if (!mask)
return -ENOMEM;
- bits = bitmap_alloc(guard.gc->ngpio, flags);
+ bits = bitmap_alloc(gc->ngpio, flags);
if (!bits) {
bitmap_free(mask);
return -ENOMEM;
}
}
- bitmap_zero(mask, guard.gc->ngpio);
+ bitmap_zero(mask, gc->ngpio);
if (!can_sleep)
- WARN_ON(guard.gc->can_sleep);
+ WARN_ON(gc->can_sleep);
do {
struct gpio_desc *desc = desc_array[i];
@@ -3839,10 +3869,10 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
i = find_next_zero_bit(array_info->set_mask,
array_size, i);
} while ((i < array_size) &&
- gpio_device_chip_cmp(desc_array[i]->gdev, guard.gc));
+ gpio_device_chip_cmp(desc_array[i]->gdev, gc));
/* push collected bits to outputs */
if (count != 0) {
- ret = gpiochip_set_multiple(guard.gc, mask, bits);
+ ret = gpiochip_set_multiple(gc, mask, bits);
if (ret)
return ret;
}
@@ -4048,7 +4078,6 @@ EXPORT_SYMBOL_GPL(gpiod_is_shared);
*/
int gpiod_to_irq(const struct gpio_desc *desc)
{
- struct gpio_device *gdev;
struct gpio_chip *gc;
int offset;
int ret;
@@ -4057,10 +4086,9 @@ int gpiod_to_irq(const struct gpio_desc *desc)
if (ret <= 0)
return -EINVAL;
- gdev = desc->gdev;
- /* FIXME Cannot use gpio_chip_guard due to const desc. */
- guard(srcu)(&gdev->srcu);
- gc = srcu_dereference(gdev->chip, &gdev->srcu);
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
+
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
if (!gc)
return -ENODEV;
@@ -5058,9 +5086,11 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
struct gpio_desc *local_desc;
int hwnum;
int ret;
+ struct gpio_chip *gc;
+ DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
- CLASS(gpio_chip_guard, guard)(desc);
- if (!guard.gc)
+ REVOCABLE_TRY_ACCESS_WITH(rev, gc);
+ if (!gc)
return -ENODEV;
if (test_and_set_bit(GPIOD_FLAG_IS_HOGGED, &desc->flags))
@@ -5068,8 +5098,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
hwnum = gpiod_hwgpio(desc);
- local_desc = gpiochip_request_own_desc(guard.gc, hwnum, name,
- lflags, dflags);
+ local_desc = gpiochip_request_own_desc(gc, hwnum, name, lflags, dflags);
if (IS_ERR(local_desc)) {
clear_bit(GPIOD_FLAG_IS_HOGGED, &desc->flags);
ret = PTR_ERR(local_desc);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index e61db3a75e84..00aa354950c9 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -224,27 +224,6 @@ struct gpio_desc {
#define gpiod_not_found(desc) (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
-struct gpio_chip_guard {
- struct gpio_device *gdev;
- struct gpio_chip *gc;
- int idx;
-};
-
-DEFINE_CLASS(gpio_chip_guard,
- struct gpio_chip_guard,
- srcu_read_unlock(&_T.gdev->srcu, _T.idx),
- ({
- struct gpio_chip_guard _guard;
-
- _guard.gdev = desc->gdev;
- _guard.idx = srcu_read_lock(&_guard.gdev->srcu);
- _guard.gc = srcu_dereference(_guard.gdev->chip,
- &_guard.gdev->srcu);
-
- _guard;
- }),
- struct gpio_desc *desc)
-
int gpiod_request(struct gpio_desc *desc, const char *label);
void gpiod_free(struct gpio_desc *desc);
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH 22/23] gpiolib: Leverage revocable for other independent lifecycle instances
2026-01-16 8:10 ` [PATCH 22/23] gpiolib: Leverage revocable for other independent lifecycle instances Tzung-Bi Shih
@ 2026-01-24 16:52 ` Johan Hovold
2026-01-26 13:58 ` Johan Hovold
0 siblings, 1 reply; 49+ messages in thread
From: Johan Hovold @ 2026-01-24 16:52 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij,
Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, Jason Gunthorpe, linux-gpio
On Fri, Jan 16, 2026 at 08:10:35AM +0000, Tzung-Bi Shih wrote:
> There are independent lifecycle instances (e.g., other drivers) can save
> a raw pointer to the struct gpio_device (e.g., via gpio_device_find())
> or struct gpio_desc (e.g., via gpio_to_desc()). In some operations,
> they have to access the underlying struct gpio_chip.
>
> Leverage revocable for them so that they don't need to handle the
> synchronization by accessing the SRCU explicitly.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
> {
> - struct gpio_device *gdev;
> struct gpio_chip *gc;
> int value;
> + DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
DEFINE_REVOCABLE() is racy and can lead to use-after-free since nothing
prevents chip_rp from being revoked and freed while the
revocable_alloc() hidden in DEFINE_REVOCABLE() is running.
>
> - /* FIXME Unable to use gpio_chip_guard due to const desc. */
> - gdev = desc->gdev;
> -
> - guard(srcu)(&gdev->srcu);
> -
> - gc = srcu_dereference(gdev->chip, &gdev->srcu);
> + REVOCABLE_TRY_ACCESS_WITH(rev, gc);
> if (!gc)
> return -ENODEV;
Johan
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 22/23] gpiolib: Leverage revocable for other independent lifecycle instances
2026-01-24 16:52 ` Johan Hovold
@ 2026-01-26 13:58 ` Johan Hovold
2026-01-27 15:56 ` Tzung-Bi Shih
0 siblings, 1 reply; 49+ messages in thread
From: Johan Hovold @ 2026-01-26 13:58 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij,
Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, Jason Gunthorpe, linux-gpio
On Sat, Jan 24, 2026 at 05:52:53PM +0100, Johan Hovold wrote:
> On Fri, Jan 16, 2026 at 08:10:35AM +0000, Tzung-Bi Shih wrote:
> > There are independent lifecycle instances (e.g., other drivers) can save
> > a raw pointer to the struct gpio_device (e.g., via gpio_device_find())
> > or struct gpio_desc (e.g., via gpio_to_desc()). In some operations,
> > they have to access the underlying struct gpio_chip.
> >
> > Leverage revocable for them so that they don't need to handle the
> > synchronization by accessing the SRCU explicitly.
> >
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
>
> > static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
> > {
> > - struct gpio_device *gdev;
> > struct gpio_chip *gc;
> > int value;
> > + DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
>
> DEFINE_REVOCABLE() is racy and can lead to use-after-free since nothing
> prevents chip_rp from being revoked and freed while the
> revocable_alloc() hidden in DEFINE_REVOCABLE() is running.
This was supposed to say "revocable_init()" (i.e. revocable_alloc()
without the memory allocation).
> >
> > - /* FIXME Unable to use gpio_chip_guard due to const desc. */
> > - gdev = desc->gdev;
> > -
> > - guard(srcu)(&gdev->srcu);
> > -
> > - gc = srcu_dereference(gdev->chip, &gdev->srcu);
> > + REVOCABLE_TRY_ACCESS_WITH(rev, gc);
> > if (!gc)
> > return -ENODEV;
Johan
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 22/23] gpiolib: Leverage revocable for other independent lifecycle instances
2026-01-26 13:58 ` Johan Hovold
@ 2026-01-27 15:56 ` Tzung-Bi Shih
0 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-27 15:56 UTC (permalink / raw)
To: Johan Hovold
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij,
Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, Jason Gunthorpe, linux-gpio
On Mon, Jan 26, 2026 at 02:58:17PM +0100, Johan Hovold wrote:
> On Sat, Jan 24, 2026 at 05:52:53PM +0100, Johan Hovold wrote:
> > On Fri, Jan 16, 2026 at 08:10:35AM +0000, Tzung-Bi Shih wrote:
> > > There are independent lifecycle instances (e.g., other drivers) can save
> > > a raw pointer to the struct gpio_device (e.g., via gpio_device_find())
> > > or struct gpio_desc (e.g., via gpio_to_desc()). In some operations,
> > > they have to access the underlying struct gpio_chip.
> > >
> > > Leverage revocable for them so that they don't need to handle the
> > > synchronization by accessing the SRCU explicitly.
> > >
> > > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> >
> > > static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
> > > {
> > > - struct gpio_device *gdev;
> > > struct gpio_chip *gc;
> > > int value;
> > > + DEFINE_REVOCABLE(rev, desc->gdev->chip_rp);
> >
> > DEFINE_REVOCABLE() is racy and can lead to use-after-free since nothing
> > prevents chip_rp from being revoked and freed while the
> > revocable_alloc() hidden in DEFINE_REVOCABLE() is running.
>
> This was supposed to say "revocable_init()" (i.e. revocable_alloc()
> without the memory allocation).
I see the issue. Thanks for identifying this.
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 23/23] gpiolib: Remove unused `chip` and `srcu` in struct gpio_device
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (21 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 22/23] gpiolib: Leverage revocable for other independent lifecycle instances Tzung-Bi Shih
@ 2026-01-16 8:10 ` Tzung-Bi Shih
2026-01-16 10:35 ` [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Bartosz Golaszewski
2026-01-19 14:21 ` (subset) " Bartosz Golaszewski
24 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-16 8:10 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij
Cc: Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, tzungbi, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, Jason Gunthorpe,
linux-gpio
`chip` and `srcu` in struct gpio_device are unused as their usages are
replaced to use revocable. Remove them.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/gpio/gpiolib.c | 26 +-------------------------
drivers/gpio/gpiolib.h | 4 ----
2 files changed, 1 insertion(+), 29 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 44915c8b6131..31f6cc27e0b7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -424,8 +424,6 @@ static int gpiochip_get_direction(struct gpio_chip *gc, unsigned int offset)
{
int ret;
- lockdep_assert_held(&gc->gpiodev->srcu);
-
if (WARN_ON(!gc->get_direction))
return -EOPNOTSUPP;
@@ -880,7 +878,6 @@ static void gpiodev_release(struct device *dev)
ida_free(&gpio_ida, gdev->id);
kfree_const(gdev->label);
kfree(gdev->descs);
- cleanup_srcu_struct(&gdev->srcu);
kfree(gdev);
}
@@ -1104,14 +1101,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
goto err_free_descs;
}
- 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;
+ goto err_free_label;
gdev->chip_rp = revocable_provider_alloc(gc);
if (!gdev->chip_rp) {
@@ -1286,8 +1278,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
err_cleanup_desc_srcu:
cleanup_srcu_struct(&gdev->desc_srcu);
-err_cleanup_gdev_srcu:
- cleanup_srcu_struct(&gdev->srcu);
err_free_label:
kfree_const(gdev->label);
err_free_descs:
@@ -1330,8 +1320,6 @@ void gpiochip_remove(struct gpio_chip *gc)
synchronize_srcu(&gpio_devices_srcu);
/* Numb the device, cancelling all outstanding operations */
- rcu_assign_pointer(gdev->chip, NULL);
- synchronize_srcu(&gdev->srcu);
revocable_provider_revoke(gdev->chip_rp);
gpio_device_teardown_shared(gdev);
gpiochip_irqchip_remove(gc);
@@ -2843,8 +2831,6 @@ static int gpiochip_direction_input(struct gpio_chip *gc, unsigned int offset)
{
int ret;
- lockdep_assert_held(&gc->gpiodev->srcu);
-
if (WARN_ON(!gc->direction_input))
return -EOPNOTSUPP;
@@ -2860,8 +2846,6 @@ static int gpiochip_direction_output(struct gpio_chip *gc, unsigned int offset,
{
int ret;
- lockdep_assert_held(&gc->gpiodev->srcu);
-
if (WARN_ON(!gc->direction_output))
return -EOPNOTSUPP;
@@ -2952,8 +2936,6 @@ static int gpiochip_set(struct gpio_chip *gc, unsigned int offset, int value)
{
int ret;
- lockdep_assert_held(&gc->gpiodev->srcu);
-
if (WARN_ON(unlikely(!gc->set)))
return -EOPNOTSUPP;
@@ -3312,8 +3294,6 @@ static int gpiochip_get(struct gpio_chip *gc, unsigned int offset)
{
int ret;
- lockdep_assert_held(&gc->gpiodev->srcu);
-
/* Make sure this is called after checking for gc->get(). */
ret = gc->get(gc, offset);
if (ret > 1)
@@ -3368,8 +3348,6 @@ static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
static int gpio_chip_get_multiple(struct gpio_chip *gc,
unsigned long *mask, unsigned long *bits)
{
- lockdep_assert_held(&gc->gpiodev->srcu);
-
if (gc->get_multiple) {
int ret;
@@ -3731,8 +3709,6 @@ static int gpiochip_set_multiple(struct gpio_chip *gc,
unsigned int i;
int ret;
- lockdep_assert_held(&gc->gpiodev->srcu);
-
if (gc->set_multiple) {
ret = gc->set_multiple(gc, mask, bits);
if (ret > 0)
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 00aa354950c9..c1952c287a64 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -28,7 +28,6 @@
* @chrdev: character device for the GPIO device
* @id: numerical ID number for the GPIO chip
* @owner: helps prevent removal of modules exporting active GPIOs
- * @chip: pointer to the corresponding gpiochip, holding static
* data for this device
* @descs: array of ngpio descriptors.
* @valid_mask: If not %NULL, holds bitmask of GPIOs which are valid to be
@@ -51,7 +50,6 @@
* process context
* @device_notifier: used to notify character device wait queues about the GPIO
* device being unregistered
- * @srcu: protects the pointer to the underlying GPIO chip
* @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
* @pin_ranges: range of pins served by the GPIO driver
*
@@ -65,7 +63,6 @@ struct gpio_device {
struct cdev chrdev;
int id;
struct module *owner;
- struct gpio_chip __rcu *chip;
struct gpio_desc *descs;
unsigned long *valid_mask;
struct srcu_struct desc_srcu;
@@ -79,7 +76,6 @@ struct gpio_device {
rwlock_t line_state_lock;
struct workqueue_struct *line_state_wq;
struct blocking_notifier_head device_notifier;
- struct srcu_struct srcu;
struct revocable_provider *chip_rp;
#ifdef CONFIG_PINCTRL
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (22 preceding siblings ...)
2026-01-16 8:10 ` [PATCH 23/23] gpiolib: Remove unused `chip` and `srcu` in struct gpio_device Tzung-Bi Shih
@ 2026-01-16 10:35 ` Bartosz Golaszewski
2026-01-16 16:07 ` Laurent Pinchart
2026-01-17 12:48 ` Tzung-Bi Shih
2026-01-19 14:21 ` (subset) " Bartosz Golaszewski
24 siblings, 2 replies; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-16 10:35 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij, Jonathan Corbet, Shuah Khan,
linux-doc, linux-kernel, chrome-platform, linux-kselftest,
Laurent Pinchart, Wolfram Sang, Simona Vetter, Dan Williams,
Jason Gunthorpe, linux-gpio
On Fri, Jan 16, 2026 at 9:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> This series transitions the UAF prevention logic within the GPIO core
> (gpiolib) to use the 'revocable' mechanism.
>
> The existing code aims to prevent UAF issues when the underlying GPIO
> chip is removed. This series replaces that custom logic with the
> generic 'revocable' API, which is designed to handle such lifecycle
> dependencies. There should be no change in behavior.
>
> This series depends on the 'revocable' API, introduced in [1]. Some
> build bots may report errors due to undefined symbols related to
> 'revocable' until the dependency is merged.
>
Hi Tzung-Bi!
Thank you for doing this and considering my suggestions from LPC. I
haven't looked at the code yet but I quickly tested the series with my
regular test-suites. The good news is: nothing is broken, every test
works fine. The bad news is: there seems to be a significant impact on
performance. With the user-space test-suite from libgpiod (for core C
library - gpiod-test) I'm seeing a consistent 40% impact on
performance. That's not really acceptable. :( I will try to bisect the
series later and see which part exactly breaks it.
I can also help you with user-space testing with libgpiod, if you need
it? Some documentation is available here:
https://libgpiod.readthedocs.io/en/latest/testing.html
> [1] https://lore.kernel.org/chrome-platform/20260116080235.350305-1-tzungbi@kernel.org
>
> Tzung-Bi Shih (23):
> gpiolib: Correct wrong kfree() usage for `kobj->name`
> gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()
> gpiolib: Fix resource leaks on errors in gpiochip_add_data_with_key()
> gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()
> gpiolib: cdev: Correct return code on memory allocation failure
>
> => The first 5 patches are fixes. They aren't directly related to the
> replacement, and should be able to apply independently.
>
Awesome, I'll make them a priority.
> gpiolib: Access `gpio_bus_type` in gpiochip_setup_dev()
> gpiolib: Remove redundant check for struct gpio_chip
> gpiolib: sysfs: Remove redundant check for struct gpio_chip
> gpiolib: Ensure struct gpio_chip for gpiochip_setup_dev()
> gpiolib: cdev: Don't check struct gpio_chip in gpio_chrdev_open()
>
> => The following 5 patches are refactors. Makes the subsequent changes
> easier or at least clear.
>
> selftests: gpio: Add gpio-cdev-uaf tests
>
> => The following patch adds kselftest cases for some classic UAF
> scenarios.
>
Thanks, these too look like v7.0 material for me. I'll try to review
these ASAP too.
Bart
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention
2026-01-16 10:35 ` [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Bartosz Golaszewski
@ 2026-01-16 16:07 ` Laurent Pinchart
2026-01-17 12:48 ` Tzung-Bi Shih
1 sibling, 0 replies; 49+ messages in thread
From: Laurent Pinchart @ 2026-01-16 16:07 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Tzung-Bi Shih, Benson Leung, Greg Kroah-Hartman,
Rafael J . Wysocki, Danilo Krummrich, Linus Walleij,
Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Wolfram Sang, Simona Vetter,
Dan Williams, Jason Gunthorpe, linux-gpio
On Fri, Jan 16, 2026 at 11:35:00AM +0100, Bartosz Golaszewski wrote:
> On Fri, Jan 16, 2026 at 9:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > This series transitions the UAF prevention logic within the GPIO core
> > (gpiolib) to use the 'revocable' mechanism.
> >
> > The existing code aims to prevent UAF issues when the underlying GPIO
> > chip is removed. This series replaces that custom logic with the
> > generic 'revocable' API, which is designed to handle such lifecycle
> > dependencies. There should be no change in behavior.
> >
> > This series depends on the 'revocable' API, introduced in [1]. Some
> > build bots may report errors due to undefined symbols related to
> > 'revocable' until the dependency is merged.
> >
>
> Hi Tzung-Bi!
>
> Thank you for doing this and considering my suggestions from LPC. I
> haven't looked at the code yet but I quickly tested the series with my
> regular test-suites. The good news is: nothing is broken, every test
> works fine. The bad news is: there seems to be a significant impact on
> performance. With the user-space test-suite from libgpiod (for core C
> library - gpiod-test) I'm seeing a consistent 40% impact on
> performance. That's not really acceptable. :( I will try to bisect the
> series later and see which part exactly breaks it.
Also, as discussed during LPC, revocable is not the right mechanism to
handle races between device removal and userspace access through cdev.
It could however be a good tool to handle races between producers and
consumers inside the kernel. I'll try to review this series to see if it
also addresses the latter.
> I can also help you with user-space testing with libgpiod, if you need
> it? Some documentation is available here:
> https://libgpiod.readthedocs.io/en/latest/testing.html
>
> > [1] https://lore.kernel.org/chrome-platform/20260116080235.350305-1-tzungbi@kernel.org
> >
> > Tzung-Bi Shih (23):
> > gpiolib: Correct wrong kfree() usage for `kobj->name`
> > gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()
> > gpiolib: Fix resource leaks on errors in gpiochip_add_data_with_key()
> > gpiolib: Fix resource leaks on errors in lineinfo_changed_notify()
> > gpiolib: cdev: Correct return code on memory allocation failure
> >
> > => The first 5 patches are fixes. They aren't directly related to the
> > replacement, and should be able to apply independently.
> >
>
> Awesome, I'll make them a priority.
>
> > gpiolib: Access `gpio_bus_type` in gpiochip_setup_dev()
> > gpiolib: Remove redundant check for struct gpio_chip
> > gpiolib: sysfs: Remove redundant check for struct gpio_chip
> > gpiolib: Ensure struct gpio_chip for gpiochip_setup_dev()
> > gpiolib: cdev: Don't check struct gpio_chip in gpio_chrdev_open()
> >
> > => The following 5 patches are refactors. Makes the subsequent changes
> > easier or at least clear.
> >
> > selftests: gpio: Add gpio-cdev-uaf tests
> >
> > => The following patch adds kselftest cases for some classic UAF
> > scenarios.
> >
>
> Thanks, these too look like v7.0 material for me. I'll try to review
> these ASAP too.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention
2026-01-16 10:35 ` [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Bartosz Golaszewski
2026-01-16 16:07 ` Laurent Pinchart
@ 2026-01-17 12:48 ` Tzung-Bi Shih
2026-01-19 8:33 ` Bartosz Golaszewski
1 sibling, 1 reply; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-17 12:48 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij, Jonathan Corbet, Shuah Khan,
linux-doc, linux-kernel, chrome-platform, linux-kselftest,
Laurent Pinchart, Wolfram Sang, Simona Vetter, Dan Williams,
Jason Gunthorpe, linux-gpio
On Fri, Jan 16, 2026 at 11:35:00AM +0100, Bartosz Golaszewski wrote:
> On Fri, Jan 16, 2026 at 9:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > This series transitions the UAF prevention logic within the GPIO core
> > (gpiolib) to use the 'revocable' mechanism.
> >
> > The existing code aims to prevent UAF issues when the underlying GPIO
> > chip is removed. This series replaces that custom logic with the
> > generic 'revocable' API, which is designed to handle such lifecycle
> > dependencies. There should be no change in behavior.
> >
> > This series depends on the 'revocable' API, introduced in [1]. Some
> > build bots may report errors due to undefined symbols related to
> > 'revocable' until the dependency is merged.
> >
>
> Hi Tzung-Bi!
>
> Thank you for doing this and considering my suggestions from LPC. I
> haven't looked at the code yet but I quickly tested the series with my
> regular test-suites. The good news is: nothing is broken, every test
> works fine. The bad news is: there seems to be a significant impact on
> performance. With the user-space test-suite from libgpiod (for core C
> library - gpiod-test) I'm seeing a consistent 40% impact on
> performance. That's not really acceptable. :( I will try to bisect the
> series later and see which part exactly breaks it.
>
> I can also help you with user-space testing with libgpiod, if you need
> it? Some documentation is available here:
> https://libgpiod.readthedocs.io/en/latest/testing.html
How to get the performance data?
I tried on libgpiod-2.2.2.tar.xz:
- ./configure --enable-tools --enable-tests
- make
- ./tests/gpiod-test
There is only TAP output. Also I don't see the difference between:
`./tests/gpiod-test` vs. `./tests/gpiod-test -m perf`.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention
2026-01-17 12:48 ` Tzung-Bi Shih
@ 2026-01-19 8:33 ` Bartosz Golaszewski
2026-01-21 4:17 ` Tzung-Bi Shih
0 siblings, 1 reply; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-19 8:33 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij, Jonathan Corbet, Shuah Khan,
linux-doc, linux-kernel, chrome-platform, linux-kselftest,
Laurent Pinchart, Wolfram Sang, Simona Vetter, Dan Williams,
Jason Gunthorpe, linux-gpio
On Sat, Jan 17, 2026 at 1:48 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Fri, Jan 16, 2026 at 11:35:00AM +0100, Bartosz Golaszewski wrote:
> > On Fri, Jan 16, 2026 at 9:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > >
> > > This series transitions the UAF prevention logic within the GPIO core
> > > (gpiolib) to use the 'revocable' mechanism.
> > >
> > > The existing code aims to prevent UAF issues when the underlying GPIO
> > > chip is removed. This series replaces that custom logic with the
> > > generic 'revocable' API, which is designed to handle such lifecycle
> > > dependencies. There should be no change in behavior.
> > >
> > > This series depends on the 'revocable' API, introduced in [1]. Some
> > > build bots may report errors due to undefined symbols related to
> > > 'revocable' until the dependency is merged.
> > >
> >
> > Hi Tzung-Bi!
> >
> > Thank you for doing this and considering my suggestions from LPC. I
> > haven't looked at the code yet but I quickly tested the series with my
> > regular test-suites. The good news is: nothing is broken, every test
> > works fine. The bad news is: there seems to be a significant impact on
> > performance. With the user-space test-suite from libgpiod (for core C
> > library - gpiod-test) I'm seeing a consistent 40% impact on
> > performance. That's not really acceptable. :( I will try to bisect the
> > series later and see which part exactly breaks it.
> >
> > I can also help you with user-space testing with libgpiod, if you need
> > it? Some documentation is available here:
> > https://libgpiod.readthedocs.io/en/latest/testing.html
>
> How to get the performance data?
>
> I tried on libgpiod-2.2.2.tar.xz:
> - ./configure --enable-tools --enable-tests
> - make
> - ./tests/gpiod-test
>
> There is only TAP output. Also I don't see the difference between:
> `./tests/gpiod-test` vs. `./tests/gpiod-test -m perf`.
Yeah, no, there's no dedicated performance measurement in GLib tests,
I just timed the test-suite and it runs 40% slower with this series.
Bartosz
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention
2026-01-19 8:33 ` Bartosz Golaszewski
@ 2026-01-21 4:17 ` Tzung-Bi Shih
2026-01-21 10:42 ` Bartosz Golaszewski
0 siblings, 1 reply; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-21 4:17 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij, Jonathan Corbet, Shuah Khan,
linux-doc, linux-kernel, chrome-platform, linux-kselftest,
Laurent Pinchart, Wolfram Sang, Simona Vetter, Dan Williams,
Jason Gunthorpe, linux-gpio
On Mon, Jan 19, 2026 at 09:33:21AM +0100, Bartosz Golaszewski wrote:
> On Sat, Jan 17, 2026 at 1:48 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > On Fri, Jan 16, 2026 at 11:35:00AM +0100, Bartosz Golaszewski wrote:
> > > On Fri, Jan 16, 2026 at 9:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > > >
> > > > This series transitions the UAF prevention logic within the GPIO core
> > > > (gpiolib) to use the 'revocable' mechanism.
> > > >
> > > > The existing code aims to prevent UAF issues when the underlying GPIO
> > > > chip is removed. This series replaces that custom logic with the
> > > > generic 'revocable' API, which is designed to handle such lifecycle
> > > > dependencies. There should be no change in behavior.
> > > >
> > > > This series depends on the 'revocable' API, introduced in [1]. Some
> > > > build bots may report errors due to undefined symbols related to
> > > > 'revocable' until the dependency is merged.
> > > >
> > >
> > > Hi Tzung-Bi!
> > >
> > > Thank you for doing this and considering my suggestions from LPC. I
> > > haven't looked at the code yet but I quickly tested the series with my
> > > regular test-suites. The good news is: nothing is broken, every test
> > > works fine. The bad news is: there seems to be a significant impact on
> > > performance. With the user-space test-suite from libgpiod (for core C
> > > library - gpiod-test) I'm seeing a consistent 40% impact on
> > > performance. That's not really acceptable. :( I will try to bisect the
> > > series later and see which part exactly breaks it.
> > >
> > > I can also help you with user-space testing with libgpiod, if you need
> > > it? Some documentation is available here:
> > > https://libgpiod.readthedocs.io/en/latest/testing.html
> >
> > How to get the performance data?
> >
> > I tried on libgpiod-2.2.2.tar.xz:
> > - ./configure --enable-tools --enable-tests
> > - make
> > - ./tests/gpiod-test
> >
> > There is only TAP output. Also I don't see the difference between:
> > `./tests/gpiod-test` vs. `./tests/gpiod-test -m perf`.
>
> Yeah, no, there's no dedicated performance measurement in GLib tests,
> I just timed the test-suite and it runs 40% slower with this series.
I think this is mostly introduced by a redundant synchronize_srcu() call in
revocable_provider_alloc(). Proposed a fix in
https://lore.kernel.org/all/20260121040204.2699886-1-tzungbi@kernel.org/.
The replacement still brings a few overhead (e.g., for allocating some in
the .open() file operations). Especially the test approach can accumulate
them.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention
2026-01-21 4:17 ` Tzung-Bi Shih
@ 2026-01-21 10:42 ` Bartosz Golaszewski
0 siblings, 0 replies; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-21 10:42 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Linus Walleij, Jonathan Corbet, Shuah Khan,
linux-doc, linux-kernel, chrome-platform, linux-kselftest,
Laurent Pinchart, Wolfram Sang, Simona Vetter, Dan Williams,
Jason Gunthorpe, linux-gpio
On Wed, Jan 21, 2026 at 5:17 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Mon, Jan 19, 2026 at 09:33:21AM +0100, Bartosz Golaszewski wrote:
> > On Sat, Jan 17, 2026 at 1:48 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > >
> > > On Fri, Jan 16, 2026 at 11:35:00AM +0100, Bartosz Golaszewski wrote:
> > > > On Fri, Jan 16, 2026 at 9:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > > > >
> > > > > This series transitions the UAF prevention logic within the GPIO core
> > > > > (gpiolib) to use the 'revocable' mechanism.
> > > > >
> > > > > The existing code aims to prevent UAF issues when the underlying GPIO
> > > > > chip is removed. This series replaces that custom logic with the
> > > > > generic 'revocable' API, which is designed to handle such lifecycle
> > > > > dependencies. There should be no change in behavior.
> > > > >
> > > > > This series depends on the 'revocable' API, introduced in [1]. Some
> > > > > build bots may report errors due to undefined symbols related to
> > > > > 'revocable' until the dependency is merged.
> > > > >
> > > >
> > > > Hi Tzung-Bi!
> > > >
> > > > Thank you for doing this and considering my suggestions from LPC. I
> > > > haven't looked at the code yet but I quickly tested the series with my
> > > > regular test-suites. The good news is: nothing is broken, every test
> > > > works fine. The bad news is: there seems to be a significant impact on
> > > > performance. With the user-space test-suite from libgpiod (for core C
> > > > library - gpiod-test) I'm seeing a consistent 40% impact on
> > > > performance. That's not really acceptable. :( I will try to bisect the
> > > > series later and see which part exactly breaks it.
> > > >
> > > > I can also help you with user-space testing with libgpiod, if you need
> > > > it? Some documentation is available here:
> > > > https://libgpiod.readthedocs.io/en/latest/testing.html
> > >
> > > How to get the performance data?
> > >
> > > I tried on libgpiod-2.2.2.tar.xz:
> > > - ./configure --enable-tools --enable-tests
> > > - make
> > > - ./tests/gpiod-test
> > >
> > > There is only TAP output. Also I don't see the difference between:
> > > `./tests/gpiod-test` vs. `./tests/gpiod-test -m perf`.
> >
> > Yeah, no, there's no dedicated performance measurement in GLib tests,
> > I just timed the test-suite and it runs 40% slower with this series.
>
> I think this is mostly introduced by a redundant synchronize_srcu() call in
> revocable_provider_alloc(). Proposed a fix in
> https://lore.kernel.org/all/20260121040204.2699886-1-tzungbi@kernel.org/.
>
> The replacement still brings a few overhead (e.g., for allocating some in
> the .open() file operations). Especially the test approach can accumulate
> them.
People do care about open()/close() performance, please see the
following discussion:
https://lore.kernel.org/all/20250311110034.53959031@erd003.prtnl/
It required us to use raw notifiers instead of atomic ones which call
rcu_synchronize() to fix the performance regression.
Bartosz
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: (subset) [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention
2026-01-16 8:10 [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Tzung-Bi Shih
` (23 preceding siblings ...)
2026-01-16 10:35 ` [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention Bartosz Golaszewski
@ 2026-01-19 14:21 ` Bartosz Golaszewski
2026-01-20 3:13 ` Tzung-Bi Shih
24 siblings, 1 reply; 49+ messages in thread
From: Bartosz Golaszewski @ 2026-01-19 14:21 UTC (permalink / raw)
To: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij,
Tzung-Bi Shih
Cc: Bartosz Golaszewski, Jonathan Corbet, Shuah Khan, linux-doc,
linux-kernel, chrome-platform, linux-kselftest, Laurent Pinchart,
Wolfram Sang, Simona Vetter, Dan Williams, linux-gpio,
Jason Gunthorpe
On Fri, 16 Jan 2026 08:10:13 +0000, Tzung-Bi Shih wrote:
> This series transitions the UAF prevention logic within the GPIO core
> (gpiolib) to use the 'revocable' mechanism.
>
> The existing code aims to prevent UAF issues when the underlying GPIO
> chip is removed. This series replaces that custom logic with the
> generic 'revocable' API, which is designed to handle such lifecycle
> dependencies. There should be no change in behavior.
>
> [...]
Tzung-Bi: I queued these two for fixes. Please resend patch 04/23 separately
so that I can queue it for v6.19 as well. 01/23 and 03/23 risk impacting a
very fragile path in GPIOLIB so any changes to it will have to wait until
v7.0-rc1 to give it a lot of time in next.
[02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()
commit: be037ec1785d76351037103ce6baddd3299606ee
[05/23] gpiolib: cdev: Correct return code on memory allocation failure
commit: c7843298bf973d4bc7f4346140661e117186decc
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: (subset) [PATCH 00/23] gpiolib: Adopt revocable mechanism for UAF prevention
2026-01-19 14:21 ` (subset) " Bartosz Golaszewski
@ 2026-01-20 3:13 ` Tzung-Bi Shih
0 siblings, 0 replies; 49+ messages in thread
From: Tzung-Bi Shih @ 2026-01-20 3:13 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Benson Leung, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Bartosz Golaszewski, Linus Walleij,
Jonathan Corbet, Shuah Khan, linux-doc, linux-kernel,
chrome-platform, linux-kselftest, Laurent Pinchart, Wolfram Sang,
Simona Vetter, Dan Williams, linux-gpio, Jason Gunthorpe
On Mon, Jan 19, 2026 at 03:21:30PM +0100, Bartosz Golaszewski wrote:
> Tzung-Bi: I queued these two for fixes. Please resend patch 04/23 separately
> so that I can queue it for v6.19 as well. 01/23 and 03/23 risk impacting a
> very fragile path in GPIOLIB so any changes to it will have to wait until
> v7.0-rc1 to give it a lot of time in next.
>
> [02/23] gpiolib: cdev: Fix resource leaks on errors in gpiolib_cdev_register()
> commit: be037ec1785d76351037103ce6baddd3299606ee
> [05/23] gpiolib: cdev: Correct return code on memory allocation failure
> commit: c7843298bf973d4bc7f4346140661e117186decc
v2 of [04/23]: https://lore.kernel.org/all/20260120030857.2144847-1-tzungbi@kernel.org
^ permalink raw reply [flat|nested] 49+ messages in thread