Re: [PATCH v12 3/3] gpio: Leverage revocable for accessing struct gpio_chip

[Tzung-Bi Shih]

On Thu, Sep 10, 2026 at 12:32:48AM -0700, Bartosz Golaszewski wrote:
> On Tue, 8 Sep 2026 15:42:23 +0200, Tzung-Bi Shih <tzungbi@xxxxxxxxxx> said:
> > The underlying chip can be removed asynchronously.  `gdev->srcu` is used
> > to ensure the synchronization before accessing `gdev->chip`.
> >
> > Revocable encapsulates the details.  Leverage revocable for accessing
> > the struct gpio_chip and remove the `gdev->srcu`.
> >
> > Tested-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Tzung-Bi Shih <tzungbi@xxxxxxxxxx>
> > ---
> 
> Could you address the following issue pointed out by sashiko?
> 
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index 66d2325bfae87..a7d3c4d9018c8 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -898,10 +897,10 @@ static void gpiodev_release(struct device *dev)
> >  	synchronize_srcu(&gdev->desc_srcu);
> >  	cleanup_srcu_struct(&gdev->desc_srcu);
> >
> > +	revocable_put(&gdev->chip_rev);
> >  	ida_free(&gpio_ida, gdev->id);
> >  	kfree_const(gdev->label);
> >  	kfree(gdev->descs);
> > -	cleanup_srcu_struct(&gdev->srcu);
> >  	kfree(gdev);
> >  }
> 
> Could this cause a use-after-free crash during concurrent device removal?
> 
> The revocable object chip_rev is embedded directly in the gpio_device struct.
> The new access cleanup pattern drops the SRCU lock before dropping the kref
> on the revocable object.
> 
> During asynchronous device removal, revocable_revoke() executes
> synchronize_srcu() to wait for readers. Because the reader drops the SRCU lock
> first, revocable_revoke() will unblock, proceed to gpio_device_put(gdev), and
> gpiodev_release() will execute kfree(gdev) while the reader thread is still
> active.
> 
> The reader thread will then attempt to call kref_put() on the chip_rev that is
> embedded inside the now-freed gdev.

While callers who use embedded allocations should properly pin the container,
the revocable primitive itself shouldn't rely on that.

I have fixed this in v13[1] by caching the flag locally inside the handle,
ensuring the teardown never touches the container's memory after unlocking
SRCU.

[1] https://lore.kernel.org/all/20260912123529.7951-2-tzungbi@xxxxxxxxxx