diff mbox series

[22/22] gpio: mark unsafe gpio_chip manipulators as deprecated

Message ID 20240130124828.14678-23-brgl@bgdev.pl
State Superseded
Headers show
Series gpio: rework locking and object life-time control | expand

Commit Message

Bartosz Golaszewski Jan. 30, 2024, 12:48 p.m. UTC
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We still have some functions that return the address of the GPIO chip
associated with the GPIO device. This is dangerous and the users should
find a better solution. Let's add appropriate comments to the kernel
docs.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Linus Walleij Jan. 31, 2024, 8:29 p.m. UTC | #1
On Tue, Jan 30, 2024 at 1:49 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> We still have some functions that return the address of the GPIO chip
> associated with the GPIO device. This is dangerous and the users should
> find a better solution. Let's add appropriate comments to the kernel
> docs.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

I'm not sure it's very easy to find a better solution for gpiod_to_chip(),
but perhaps also add this as a work item to the TODO file? We can certainly
try to get rid of <linux/gpio.h> before we need to look into fixing this...

gpiod_device_get_label() should be easy to fix:
linus@lino:~/linux$ git grep gpio_device_get_label
drivers/gpio/gpiolib.c: * gpio_device_get_label() - Get the label of
this GPIO device
drivers/gpio/gpiolib.c:const char *gpio_device_get_label(struct
gpio_device *gdev)
drivers/gpio/gpiolib.c:EXPORT_SYMBOL(gpio_device_get_label);
drivers/pinctrl/core.c:                            gpio_device_get_label(gdev));
include/linux/gpio/driver.h:const char *gpio_device_get_label(struct
gpio_device *gdev);

We only created that problem for ourselves... It should be removed
from <linux/gpio/driver.h>.

Anyway:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Bartosz Golaszewski Feb. 1, 2024, 9:14 a.m. UTC | #2
On Wed, Jan 31, 2024 at 9:29 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Jan 30, 2024 at 1:49 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > We still have some functions that return the address of the GPIO chip
> > associated with the GPIO device. This is dangerous and the users should
> > find a better solution. Let's add appropriate comments to the kernel
> > docs.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> I'm not sure it's very easy to find a better solution for gpiod_to_chip(),
> but perhaps also add this as a work item to the TODO file? We can certainly
> try to get rid of <linux/gpio.h> before we need to look into fixing this...

I will get to revising the TODO at some point hopefully.

>
> gpiod_device_get_label() should be easy to fix:

This is gone already! Check v6.8-rc1.

Bart

> linus@lino:~/linux$ git grep gpio_device_get_label
> drivers/gpio/gpiolib.c: * gpio_device_get_label() - Get the label of
> this GPIO device
> drivers/gpio/gpiolib.c:const char *gpio_device_get_label(struct
> gpio_device *gdev)
> drivers/gpio/gpiolib.c:EXPORT_SYMBOL(gpio_device_get_label);
> drivers/pinctrl/core.c:                            gpio_device_get_label(gdev));
> include/linux/gpio/driver.h:const char *gpio_device_get_label(struct
> gpio_device *gdev);
>
> We only created that problem for ourselves... It should be removed
> from <linux/gpio/driver.h>.
>
> Anyway:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Yours,
> Linus Walleij
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5741bbfdc178..7ecdd8cc39c5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -211,6 +211,11 @@  EXPORT_SYMBOL_GPL(desc_to_gpio);
 /**
  * gpiod_to_chip - Return the GPIO chip to which a GPIO descriptor belongs
  * @desc:	descriptor to return the chip of
+ *
+ * *DEPRECATED*
+ * This function is unsafe and should not be used. Using the chip address
+ * without taking the SRCU read lock may result in dereferencing a dangling
+ * pointer.
  */
 struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
 {
@@ -275,6 +280,7 @@  EXPORT_SYMBOL(gpio_device_get_label);
  * Returns:
  * Address of the GPIO chip backing this device.
  *
+ * *DEPRECATED*
  * Until we can get rid of all non-driver users of struct gpio_chip, we must
  * provide a way of retrieving the pointer to it from struct gpio_device. This
  * is *NOT* safe as the GPIO API is considered to be hot-unpluggable and the