Skip to content

Commit cfb4be1

Browse files
committed
Merge tag 'gpio-fixes-for-v6.9' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux
Pull gpio fixes from Bartosz Golaszewski: "Some last-minute fixes for this release from the GPIO subsystem. The first two address a regression in performance reported to me after the conversion to using SRCU in GPIOLIB that was merged during the v6.9 merge window. The second patch is not technically a fix but since after the first one we no longer need to use a per-descriptor SRCU struct, I think it's worth to simplify the code before it gets released on Sunday. The next two commits fix two memory issues: one use-after-free bug and one instance of possibly leaking kernel stack memory to user-space. Summary: - fix a performance regression in GPIO requesting and releasing after the conversion to SRCU - fix a use-after-free bug due to a race-condition - fix leaking stack memory to user-space in a GPIO uABI corner case" * tag 'gpio-fixes-for-v6.9' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux: gpiolib: cdev: fix uninitialised kfifo gpiolib: cdev: Fix use after free in lineinfo_changed_notify gpiolib: use a single SRCU struct for all GPIO descriptors gpiolib: fix the speed of descriptor label setting with SRCU
2 parents f4345f0 + ee0166b commit cfb4be1

File tree

3 files changed

+63
-30
lines changed

3 files changed

+63
-30
lines changed

drivers/gpio/gpiolib-cdev.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,6 +1193,8 @@ static int edge_detector_update(struct line *line,
11931193
struct gpio_v2_line_config *lc,
11941194
unsigned int line_idx, u64 edflags)
11951195
{
1196+
u64 eflags;
1197+
int ret;
11961198
u64 active_edflags = READ_ONCE(line->edflags);
11971199
unsigned int debounce_period_us =
11981200
gpio_v2_line_config_debounce_period(lc, line_idx);
@@ -1204,6 +1206,18 @@ static int edge_detector_update(struct line *line,
12041206
/* sw debounced and still will be...*/
12051207
if (debounce_period_us && READ_ONCE(line->sw_debounced)) {
12061208
line_set_debounce_period(line, debounce_period_us);
1209+
/*
1210+
* ensure event fifo is initialised if edge detection
1211+
* is now enabled.
1212+
*/
1213+
eflags = edflags & GPIO_V2_LINE_EDGE_FLAGS;
1214+
if (eflags && !kfifo_initialized(&line->req->events)) {
1215+
ret = kfifo_alloc(&line->req->events,
1216+
line->req->event_buffer_size,
1217+
GFP_KERNEL);
1218+
if (ret)
1219+
return ret;
1220+
}
12071221
return 0;
12081222
}
12091223

@@ -2351,7 +2365,7 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
23512365

23522366
dflags = READ_ONCE(desc->flags);
23532367

2354-
scoped_guard(srcu, &desc->srcu) {
2368+
scoped_guard(srcu, &desc->gdev->desc_srcu) {
23552369
label = gpiod_get_label(desc);
23562370
if (label && test_bit(FLAG_REQUESTED, &dflags))
23572371
strscpy(info->consumer, label,
@@ -2799,11 +2813,11 @@ static int gpio_chrdev_release(struct inode *inode, struct file *file)
27992813
struct gpio_chardev_data *cdev = file->private_data;
28002814
struct gpio_device *gdev = cdev->gdev;
28012815

2802-
bitmap_free(cdev->watched_lines);
28032816
blocking_notifier_chain_unregister(&gdev->device_notifier,
28042817
&cdev->device_unregistered_nb);
28052818
blocking_notifier_chain_unregister(&gdev->line_state_notifier,
28062819
&cdev->lineinfo_changed_nb);
2820+
bitmap_free(cdev->watched_lines);
28072821
gpio_device_put(gdev);
28082822
kfree(cdev);
28092823

drivers/gpio/gpiolib.c

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -101,30 +101,44 @@ static bool gpiolib_initialized;
101101

102102
const char *gpiod_get_label(struct gpio_desc *desc)
103103
{
104+
struct gpio_desc_label *label;
104105
unsigned long flags;
105106

106107
flags = READ_ONCE(desc->flags);
107108
if (test_bit(FLAG_USED_AS_IRQ, &flags) &&
108109
!test_bit(FLAG_REQUESTED, &flags))
109110
return "interrupt";
110111

111-
return test_bit(FLAG_REQUESTED, &flags) ?
112-
srcu_dereference(desc->label, &desc->srcu) : NULL;
112+
if (!test_bit(FLAG_REQUESTED, &flags))
113+
return NULL;
114+
115+
label = srcu_dereference_check(desc->label, &desc->gdev->desc_srcu,
116+
srcu_read_lock_held(&desc->gdev->desc_srcu));
117+
118+
return label->str;
119+
}
120+
121+
static void desc_free_label(struct rcu_head *rh)
122+
{
123+
kfree(container_of(rh, struct gpio_desc_label, rh));
113124
}
114125

115126
static int desc_set_label(struct gpio_desc *desc, const char *label)
116127
{
117-
const char *new = NULL, *old;
128+
struct gpio_desc_label *new = NULL, *old;
118129

119130
if (label) {
120-
new = kstrdup_const(label, GFP_KERNEL);
131+
new = kzalloc(struct_size(new, str, strlen(label) + 1),
132+
GFP_KERNEL);
121133
if (!new)
122134
return -ENOMEM;
135+
136+
strcpy(new->str, label);
123137
}
124138

125139
old = rcu_replace_pointer(desc->label, new, 1);
126-
synchronize_srcu(&desc->srcu);
127-
kfree_const(old);
140+
if (old)
141+
call_srcu(&desc->gdev->desc_srcu, &old->rh, desc_free_label);
128142

129143
return 0;
130144
}
@@ -695,10 +709,10 @@ EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
695709
static void gpiodev_release(struct device *dev)
696710
{
697711
struct gpio_device *gdev = to_gpio_device(dev);
698-
unsigned int i;
699712

700-
for (i = 0; i < gdev->ngpio; i++)
701-
cleanup_srcu_struct(&gdev->descs[i].srcu);
713+
/* Call pending kfree()s for descriptor labels. */
714+
synchronize_srcu(&gdev->desc_srcu);
715+
cleanup_srcu_struct(&gdev->desc_srcu);
702716

703717
ida_free(&gpio_ida, gdev->id);
704718
kfree_const(gdev->label);
@@ -975,30 +989,30 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
975989
if (ret)
976990
goto err_remove_from_list;
977991

992+
ret = init_srcu_struct(&gdev->desc_srcu);
993+
if (ret)
994+
goto err_cleanup_gdev_srcu;
995+
978996
#ifdef CONFIG_PINCTRL
979997
INIT_LIST_HEAD(&gdev->pin_ranges);
980998
#endif
981999

9821000
if (gc->names) {
9831001
ret = gpiochip_set_desc_names(gc);
9841002
if (ret)
985-
goto err_cleanup_gdev_srcu;
1003+
goto err_cleanup_desc_srcu;
9861004
}
9871005
ret = gpiochip_set_names(gc);
9881006
if (ret)
989-
goto err_cleanup_gdev_srcu;
1007+
goto err_cleanup_desc_srcu;
9901008

9911009
ret = gpiochip_init_valid_mask(gc);
9921010
if (ret)
993-
goto err_cleanup_gdev_srcu;
1011+
goto err_cleanup_desc_srcu;
9941012

9951013
for (desc_index = 0; desc_index < gc->ngpio; desc_index++) {
9961014
struct gpio_desc *desc = &gdev->descs[desc_index];
9971015

998-
ret = init_srcu_struct(&desc->srcu);
999-
if (ret)
1000-
goto err_cleanup_desc_srcu;
1001-
10021016
if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index)) {
10031017
assign_bit(FLAG_IS_OUT,
10041018
&desc->flags, !gc->get_direction(gc, desc_index));
@@ -1010,7 +1024,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
10101024

10111025
ret = of_gpiochip_add(gc);
10121026
if (ret)
1013-
goto err_cleanup_desc_srcu;
1027+
goto err_free_valid_mask;
10141028

10151029
ret = gpiochip_add_pin_ranges(gc);
10161030
if (ret)
@@ -1057,10 +1071,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
10571071
gpiochip_remove_pin_ranges(gc);
10581072
err_remove_of_chip:
10591073
of_gpiochip_remove(gc);
1060-
err_cleanup_desc_srcu:
1061-
while (desc_index--)
1062-
cleanup_srcu_struct(&gdev->descs[desc_index].srcu);
1074+
err_free_valid_mask:
10631075
gpiochip_free_valid_mask(gc);
1076+
err_cleanup_desc_srcu:
1077+
cleanup_srcu_struct(&gdev->desc_srcu);
10641078
err_cleanup_gdev_srcu:
10651079
cleanup_srcu_struct(&gdev->srcu);
10661080
err_remove_from_list:
@@ -2390,7 +2404,7 @@ char *gpiochip_dup_line_label(struct gpio_chip *gc, unsigned int offset)
23902404
if (!test_bit(FLAG_REQUESTED, &desc->flags))
23912405
return NULL;
23922406

2393-
guard(srcu)(&desc->srcu);
2407+
guard(srcu)(&desc->gdev->desc_srcu);
23942408

23952409
label = kstrdup(gpiod_get_label(desc), GFP_KERNEL);
23962410
if (!label)
@@ -4781,7 +4795,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
47814795
}
47824796

47834797
for_each_gpio_desc(gc, desc) {
4784-
guard(srcu)(&desc->srcu);
4798+
guard(srcu)(&desc->gdev->desc_srcu);
47854799
if (test_bit(FLAG_REQUESTED, &desc->flags)) {
47864800
gpiod_get_direction(desc);
47874801
is_out = test_bit(FLAG_IS_OUT, &desc->flags);

drivers/gpio/gpiolib.h

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
* @chip: pointer to the corresponding gpiochip, holding static
3232
* data for this device
3333
* @descs: array of ngpio descriptors.
34+
* @desc_srcu: ensures consistent state of GPIO descriptors exposed to users
3435
* @ngpio: the number of GPIO lines on this GPIO device, equal to the size
3536
* of the @descs array.
3637
* @can_sleep: indicate whether the GPIO chip driver's callbacks can sleep
@@ -61,6 +62,7 @@ struct gpio_device {
6162
struct module *owner;
6263
struct gpio_chip __rcu *chip;
6364
struct gpio_desc *descs;
65+
struct srcu_struct desc_srcu;
6466
int base;
6567
u16 ngpio;
6668
bool can_sleep;
@@ -137,6 +139,11 @@ int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
137139

138140
void gpiod_line_state_notify(struct gpio_desc *desc, unsigned long action);
139141

142+
struct gpio_desc_label {
143+
struct rcu_head rh;
144+
char str[];
145+
};
146+
140147
/**
141148
* struct gpio_desc - Opaque descriptor for a GPIO
142149
*
@@ -145,7 +152,6 @@ void gpiod_line_state_notify(struct gpio_desc *desc, unsigned long action);
145152
* @label: Name of the consumer
146153
* @name: Line name
147154
* @hog: Pointer to the device node that hogs this line (if any)
148-
* @srcu: SRCU struct protecting the label pointer.
149155
*
150156
* These are obtained using gpiod_get() and are preferable to the old
151157
* integer-based handles.
@@ -177,13 +183,12 @@ struct gpio_desc {
177183
#define FLAG_EVENT_CLOCK_HTE 19 /* GPIO CDEV reports hardware timestamps in events */
178184

179185
/* Connection label */
180-
const char __rcu *label;
186+
struct gpio_desc_label __rcu *label;
181187
/* Name of the GPIO */
182188
const char *name;
183189
#ifdef CONFIG_OF_DYNAMIC
184190
struct device_node *hog;
185191
#endif
186-
struct srcu_struct srcu;
187192
};
188193

189194
#define gpiod_not_found(desc) (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
@@ -251,23 +256,23 @@ static inline int gpio_chip_hwgpio(const struct gpio_desc *desc)
251256

252257
#define gpiod_err(desc, fmt, ...) \
253258
do { \
254-
scoped_guard(srcu, &desc->srcu) { \
259+
scoped_guard(srcu, &desc->gdev->desc_srcu) { \
255260
pr_err("gpio-%d (%s): " fmt, desc_to_gpio(desc), \
256261
gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \
257262
} \
258263
} while (0)
259264

260265
#define gpiod_warn(desc, fmt, ...) \
261266
do { \
262-
scoped_guard(srcu, &desc->srcu) { \
267+
scoped_guard(srcu, &desc->gdev->desc_srcu) { \
263268
pr_warn("gpio-%d (%s): " fmt, desc_to_gpio(desc), \
264269
gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \
265270
} \
266271
} while (0)
267272

268273
#define gpiod_dbg(desc, fmt, ...) \
269274
do { \
270-
scoped_guard(srcu, &desc->srcu) { \
275+
scoped_guard(srcu, &desc->gdev->desc_srcu) { \
271276
pr_debug("gpio-%d (%s): " fmt, desc_to_gpio(desc), \
272277
gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \
273278
} \

0 commit comments

Comments
 (0)