gpio: protect the descriptor label with SRCU

In order to ensure that the label is not freed while it's being
accessed, let's protect it with SRCU and synchronize it everytime it's
changed.

Let's modify desc_set_label() to manage the memory used for the label as
it can only be freed once synchronize_srcu() returns.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 2c0a0700762d8d94e3be0ac43ad232f97a1392c8..75f4912339a6598a18a6a75cefff49cea70bc230 100644 (file)
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2297,6 +2297,7 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
 {
        struct gpio_chip *gc = desc->gdev->chip;
        unsigned long dflags;
+       const char *label;
 
        memset(info, 0, sizeof(*info));
        info->offset = gpio_chip_hwgpio(desc);
@@ -2305,9 +2306,12 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
                if (desc->name)
                        strscpy(info->name, desc->name, sizeof(info->name));
 
-               if (gpiod_get_label(desc))
-                       strscpy(info->consumer, gpiod_get_label(desc),
-                               sizeof(info->consumer));
+               scoped_guard(srcu, &desc->srcu) {
+                       label = gpiod_get_label(desc);
+                       if (label)
+                               strscpy(info->consumer, label,
+                                       sizeof(info->consumer));
+               }
 
                dflags = READ_ONCE(desc->flags);
        }

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 78b9e9dc268a61138435a6abe99258fea3fdc40e..c72f3ffbe6b454c1b1cfbaedccae734c25c36de0 100644 (file)
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -114,12 +114,26 @@ const char *gpiod_get_label(struct gpio_desc *desc)
            !test_bit(FLAG_REQUESTED, &flags))
                return "interrupt";
 
-       return test_bit(FLAG_REQUESTED, &flags) ? desc->label : NULL;
+       return test_bit(FLAG_REQUESTED, &flags) ?
+                       rcu_dereference(desc->label) : NULL;
 }
 
-static inline void desc_set_label(struct gpio_desc *d, const char *label)
+static int desc_set_label(struct gpio_desc *desc, const char *label)
 {
-       d->label = label;
+       const char *new = NULL, *old;
+
+       if (label) {
+               /* FIXME: make this GFP_KERNEL once the spinlock is out. */
+               new = kstrdup_const(label, GFP_ATOMIC);
+               if (!new)
+                       return -ENOMEM;
+       }
+
+       old = rcu_replace_pointer(desc->label, new, 1);
+       synchronize_srcu(&desc->srcu);
+       kfree_const(old);
+
+       return 0;
 }
 
 /**
@@ -2229,9 +2243,7 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
         * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
         */
 
-       if (test_and_set_bit(FLAG_REQUESTED, &desc->flags) == 0) {
-               desc_set_label(desc, label ? : "?");
-       } else {
+       if (test_and_set_bit(FLAG_REQUESTED, &desc->flags)) {
                ret = -EBUSY;
                goto out_free_unlock;
        }
@@ -2259,6 +2271,13 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
                spin_lock_irqsave(&gpio_lock, flags);
        }
        spin_unlock_irqrestore(&gpio_lock, flags);
+
+       ret = desc_set_label(desc, label ? : "?");
+       if (ret) {
+               clear_bit(FLAG_REQUESTED, &desc->flags);
+               return ret;
+       }
+
        return 0;
 
 out_free_unlock:
@@ -2343,8 +2362,6 @@ static bool gpiod_free_commit(struct gpio_desc *desc)
                        gc->free(gc, gpio_chip_hwgpio(desc));
                        spin_lock_irqsave(&gpio_lock, flags);
                }
-               kfree_const(desc->label);
-               desc_set_label(desc, NULL);
                clear_bit(FLAG_ACTIVE_LOW, &desc->flags);
                clear_bit(FLAG_REQUESTED, &desc->flags);
                clear_bit(FLAG_OPEN_DRAIN, &desc->flags);
@@ -2362,6 +2379,7 @@ static bool gpiod_free_commit(struct gpio_desc *desc)
        }
 
        spin_unlock_irqrestore(&gpio_lock, flags);
+       desc_set_label(desc, NULL);
        gpiod_line_state_notify(desc, GPIOLINE_CHANGED_RELEASED);
 
        return ret;
@@ -2409,6 +2427,8 @@ char *gpiochip_dup_line_label(struct gpio_chip *gc, unsigned int offset)
        if (!test_bit(FLAG_REQUESTED, &desc->flags))
                return NULL;
 
+       guard(srcu)(&desc->srcu);
+
        /*
         * FIXME: Once we mark gpiod_direction_input/output() and
         * gpiod_get_direction() with might_sleep(), we'll be able to protect
@@ -3520,16 +3540,8 @@ EXPORT_SYMBOL_GPL(gpiod_cansleep);
 int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name)
 {
        VALIDATE_DESC(desc);
-       if (name) {
-               name = kstrdup_const(name, GFP_KERNEL);
-               if (!name)
-                       return -ENOMEM;
-       }
 
-       kfree_const(desc->label);
-       desc_set_label(desc, name);
-
-       return 0;
+       return desc_set_label(desc, name);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_consumer_name);
 
@@ -4739,6 +4751,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
        int value;
 
        for_each_gpio_desc(gc, desc) {
+               guard(srcu)(&desc->srcu);
                if (test_bit(FLAG_REQUESTED, &desc->flags)) {
                        gpiod_get_direction(desc);
                        is_out = test_bit(FLAG_IS_OUT, &desc->flags);

diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 6e14b629c48ba9865924617ceddc35a069e21806..d2e73eea9e922687664fcf806f588babcd2a5b11 100644 (file)
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -180,7 +180,7 @@ struct gpio_desc {
 #define FLAG_EVENT_CLOCK_HTE           19 /* GPIO CDEV reports hardware timestamps in events */
 
        /* Connection label */
-       const char              *label;
+       const char __rcu        *label;
        /* Name of the GPIO */
        const char              *name;
 #ifdef CONFIG_OF_DYNAMIC
@@ -223,15 +223,29 @@ static inline int gpio_chip_hwgpio(const struct gpio_desc *desc)
 
 /* With descriptor prefix */
 
-#define gpiod_err(desc, fmt, ...)                                             \
-       pr_err("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label ? : "?",  \
-                ##__VA_ARGS__)
-#define gpiod_warn(desc, fmt, ...)                                            \
-       pr_warn("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label ? : "?", \
-                ##__VA_ARGS__)
-#define gpiod_dbg(desc, fmt, ...)                                             \
-       pr_debug("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label ? : "?",\
-                ##__VA_ARGS__)
+#define gpiod_err(desc, fmt, ...) \
+do { \
+       scoped_guard(srcu, &desc->srcu) { \
+               pr_err("gpio-%d (%s): " fmt, desc_to_gpio(desc), \
+                      gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \
+       } \
+} while (0)
+
+#define gpiod_warn(desc, fmt, ...) \
+do { \
+       scoped_guard(srcu, &desc->srcu) { \
+               pr_warn("gpio-%d (%s): " fmt, desc_to_gpio(desc), \
+                       gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \
+       } \
+} while (0)
+
+#define gpiod_dbg(desc, fmt, ...) \
+do { \
+       scoped_guard(srcu, &desc->srcu) { \
+               pr_debug("gpio-%d (%s): " fmt, desc_to_gpio(desc), \
+                        gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \
+       } \
+} while (0)
 
 /* With chip prefix */