Skip to content

Commit

Permalink
gpio: protect the descriptor label with SRCU
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Bartosz Golaszewski committed Feb 12, 2024
1 parent be711ca commit 1f2bcb8
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 30 deletions.
10 changes: 7 additions & 3 deletions drivers/gpio/gpiolib-cdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down
47 changes: 30 additions & 17 deletions drivers/gpio/gpiolib.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
34 changes: 24 additions & 10 deletions drivers/gpio/gpiolib.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 */

Expand Down

0 comments on commit 1f2bcb8

Please sign in to comment.