[PATCH v3 07/24] gpio: protect the descriptor label with SRCU
From: Bartosz Golaszewski
Date: Thu Feb 08 2024 - 05:01:35 EST
From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
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@xxxxxxxxxx>
Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
---
drivers/gpio/gpiolib-cdev.c | 10 +++++---
drivers/gpio/gpiolib.c | 47 +++++++++++++++++++++++--------------
drivers/gpio/gpiolib.h | 34 +++++++++++++++++++--------
3 files changed, 61 insertions(+), 30 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 2c0a0700762d..75f4912339a6 100644
--- 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 b26d39bc3389..a87ab8f013c4 100644
--- 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 6e14b629c48b..d2e73eea9e92 100644
--- 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 */
--
2.40.1