Commit 1f2bcb8c authored by Bartosz Golaszewski's avatar Bartosz Golaszewski
Browse files

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: default avatarBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: default avatarLinus Walleij <linus.walleij@linaro.org>
Acked-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
parent be711caa

drivers/gpio/gpiolib-cdev.c

+7 −3
Original line number Diff line number Diff line
@@ -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),

		scoped_guard(srcu, &desc->srcu) {

			label = gpiod_get_label(desc);

			if (label)

				strscpy(info->consumer, label,

					sizeof(info->consumer));

		}



		dflags = READ_ONCE(desc->flags);

	}

drivers/gpio/gpiolib.c

+30 −17
Original line number Diff line number Diff line
@@ -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);

drivers/gpio/gpiolib.h

+24 −10
Original line number Diff line number Diff line
@@ -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
@@ -224,14 +224,28 @@ 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__)

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, ...) \

	pr_warn("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label ? : "?", \

		 ##__VA_ARGS__)

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, ...) \

	pr_debug("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label ? : "?",\

		 ##__VA_ARGS__)

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 */