Skip to content

Commit a7ac22d

Browse files
Paweł NarewskiBartosz Golaszewski
authored andcommitted
gpiolib: fix race condition for gdev->srcu
If two drivers were calling gpiochip_add_data_with_key(), one may be traversing the srcu-protected list in gpio_name_to_desc(), meanwhile other has just added its gdev in gpiodev_add_to_list_unlocked(). This creates a non-mutexed and non-protected timeframe, when one instance is dereferencing and using &gdev->srcu, before the other has initialized it, resulting in crash: [ 4.935481] Unable to handle kernel paging request at virtual address ffff800272bcc000 [ 4.943396] Mem abort info: [ 4.943400] ESR = 0x0000000096000005 [ 4.943403] EC = 0x25: DABT (current EL), IL = 32 bits [ 4.943407] SET = 0, FnV = 0 [ 4.943410] EA = 0, S1PTW = 0 [ 4.943413] FSC = 0x05: level 1 translation fault [ 4.943416] Data abort info: [ 4.943418] ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000 [ 4.946220] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 4.955261] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 4.955268] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000038e6c000 [ 4.961449] [ffff800272bcc000] pgd=0000000000000000 [ 4.969203] , p4d=1000000039739003 [ 4.979730] , pud=0000000000000000 [ 4.980210] phandle (CPU): 0x0000005e, phandle (BE): 0x5e000000 for node "reset" [ 4.991736] Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP ... [ 5.121359] pc : __srcu_read_lock+0x44/0x98 [ 5.131091] lr : gpio_name_to_desc+0x60/0x1a0 [ 5.153671] sp : ffff8000833bb430 [ 5.298440] [ 5.298443] Call trace: [ 5.298445] __srcu_read_lock+0x44/0x98 [ 5.309484] gpio_name_to_desc+0x60/0x1a0 [ 5.320692] gpiochip_add_data_with_key+0x488/0xf00 5.946419] ---[ end trace 0000000000000000 ]--- Move initialization code for gdev fields before it is added to gpio_devices, with adjacent initialization code. Adjust goto statements to reflect modified order of operations Fixes: 47d8b4c ("gpio: add SRCU infrastructure to struct gpio_device") Reviewed-by: Jakub Lewalski <jakub.lewalski@nokia.com> Signed-off-by: Paweł Narewski <pawel.narewski@nokia.com> [Bartosz: fixed a build issue, removed stray newline] Link: https://lore.kernel.org/r/20251224082641.10769-1-bartosz.golaszewski@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
1 parent 4941648 commit a7ac22d

1 file changed

Lines changed: 20 additions & 20 deletions

File tree

drivers/gpio/gpiolib.c

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,6 +1105,18 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
11051105
gdev->ngpio = gc->ngpio;
11061106
gdev->can_sleep = gc->can_sleep;
11071107

1108+
rwlock_init(&gdev->line_state_lock);
1109+
RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
1110+
BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
1111+
1112+
ret = init_srcu_struct(&gdev->srcu);
1113+
if (ret)
1114+
goto err_free_label;
1115+
1116+
ret = init_srcu_struct(&gdev->desc_srcu);
1117+
if (ret)
1118+
goto err_cleanup_gdev_srcu;
1119+
11081120
scoped_guard(mutex, &gpio_devices_lock) {
11091121
/*
11101122
* TODO: this allocates a Linux GPIO number base in the global
@@ -1119,7 +1131,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
11191131
if (base < 0) {
11201132
ret = base;
11211133
base = 0;
1122-
goto err_free_label;
1134+
goto err_cleanup_desc_srcu;
11231135
}
11241136

11251137
/*
@@ -1139,22 +1151,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
11391151
ret = gpiodev_add_to_list_unlocked(gdev);
11401152
if (ret) {
11411153
gpiochip_err(gc, "GPIO integer space overlap, cannot add chip\n");
1142-
goto err_free_label;
1154+
goto err_cleanup_desc_srcu;
11431155
}
11441156
}
11451157

1146-
rwlock_init(&gdev->line_state_lock);
1147-
RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
1148-
BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
1149-
1150-
ret = init_srcu_struct(&gdev->srcu);
1151-
if (ret)
1152-
goto err_remove_from_list;
1153-
1154-
ret = init_srcu_struct(&gdev->desc_srcu);
1155-
if (ret)
1156-
goto err_cleanup_gdev_srcu;
1157-
11581158
#ifdef CONFIG_PINCTRL
11591159
INIT_LIST_HEAD(&gdev->pin_ranges);
11601160
#endif
@@ -1164,11 +1164,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
11641164

11651165
ret = gpiochip_set_names(gc);
11661166
if (ret)
1167-
goto err_cleanup_desc_srcu;
1167+
goto err_remove_from_list;
11681168

11691169
ret = gpiochip_init_valid_mask(gc);
11701170
if (ret)
1171-
goto err_cleanup_desc_srcu;
1171+
goto err_remove_from_list;
11721172

11731173
for (desc_index = 0; desc_index < gc->ngpio; desc_index++) {
11741174
struct gpio_desc *desc = &gdev->descs[desc_index];
@@ -1248,10 +1248,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
12481248
of_gpiochip_remove(gc);
12491249
err_free_valid_mask:
12501250
gpiochip_free_valid_mask(gc);
1251-
err_cleanup_desc_srcu:
1252-
cleanup_srcu_struct(&gdev->desc_srcu);
1253-
err_cleanup_gdev_srcu:
1254-
cleanup_srcu_struct(&gdev->srcu);
12551251
err_remove_from_list:
12561252
scoped_guard(mutex, &gpio_devices_lock)
12571253
list_del_rcu(&gdev->list);
@@ -1261,6 +1257,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
12611257
gpio_device_put(gdev);
12621258
goto err_print_message;
12631259
}
1260+
err_cleanup_desc_srcu:
1261+
cleanup_srcu_struct(&gdev->desc_srcu);
1262+
err_cleanup_gdev_srcu:
1263+
cleanup_srcu_struct(&gdev->srcu);
12641264
err_free_label:
12651265
kfree_const(gdev->label);
12661266
err_free_descs:

0 commit comments

Comments
 (0)