Skip to content

Commit fb674c8

Browse files
Paweł Narewskigregkh
authored andcommitted
gpiolib: fix race condition for gdev->srcu
[ Upstream commit a7ac22d ] 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> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent aaa24ee commit fb674c8

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
@@ -1091,6 +1091,18 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
10911091
gdev->ngpio = gc->ngpio;
10921092
gdev->can_sleep = gc->can_sleep;
10931093

1094+
rwlock_init(&gdev->line_state_lock);
1095+
RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
1096+
BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
1097+
1098+
ret = init_srcu_struct(&gdev->srcu);
1099+
if (ret)
1100+
goto err_free_label;
1101+
1102+
ret = init_srcu_struct(&gdev->desc_srcu);
1103+
if (ret)
1104+
goto err_cleanup_gdev_srcu;
1105+
10941106
scoped_guard(mutex, &gpio_devices_lock) {
10951107
/*
10961108
* TODO: this allocates a Linux GPIO number base in the global
@@ -1105,7 +1117,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
11051117
if (base < 0) {
11061118
ret = base;
11071119
base = 0;
1108-
goto err_free_label;
1120+
goto err_cleanup_desc_srcu;
11091121
}
11101122

11111123
/*
@@ -1125,22 +1137,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
11251137
ret = gpiodev_add_to_list_unlocked(gdev);
11261138
if (ret) {
11271139
gpiochip_err(gc, "GPIO integer space overlap, cannot add chip\n");
1128-
goto err_free_label;
1140+
goto err_cleanup_desc_srcu;
11291141
}
11301142
}
11311143

1132-
rwlock_init(&gdev->line_state_lock);
1133-
RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
1134-
BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
1135-
1136-
ret = init_srcu_struct(&gdev->srcu);
1137-
if (ret)
1138-
goto err_remove_from_list;
1139-
1140-
ret = init_srcu_struct(&gdev->desc_srcu);
1141-
if (ret)
1142-
goto err_cleanup_gdev_srcu;
1143-
11441144
#ifdef CONFIG_PINCTRL
11451145
INIT_LIST_HEAD(&gdev->pin_ranges);
11461146
#endif
@@ -1150,11 +1150,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
11501150

11511151
ret = gpiochip_set_names(gc);
11521152
if (ret)
1153-
goto err_cleanup_desc_srcu;
1153+
goto err_remove_from_list;
11541154

11551155
ret = gpiochip_init_valid_mask(gc);
11561156
if (ret)
1157-
goto err_cleanup_desc_srcu;
1157+
goto err_remove_from_list;
11581158

11591159
for (desc_index = 0; desc_index < gc->ngpio; desc_index++) {
11601160
struct gpio_desc *desc = &gdev->descs[desc_index];
@@ -1227,10 +1227,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
12271227
of_gpiochip_remove(gc);
12281228
err_free_valid_mask:
12291229
gpiochip_free_valid_mask(gc);
1230-
err_cleanup_desc_srcu:
1231-
cleanup_srcu_struct(&gdev->desc_srcu);
1232-
err_cleanup_gdev_srcu:
1233-
cleanup_srcu_struct(&gdev->srcu);
12341230
err_remove_from_list:
12351231
scoped_guard(mutex, &gpio_devices_lock)
12361232
list_del_rcu(&gdev->list);
@@ -1240,6 +1236,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
12401236
gpio_device_put(gdev);
12411237
goto err_print_message;
12421238
}
1239+
err_cleanup_desc_srcu:
1240+
cleanup_srcu_struct(&gdev->desc_srcu);
1241+
err_cleanup_gdev_srcu:
1242+
cleanup_srcu_struct(&gdev->srcu);
12431243
err_free_label:
12441244
kfree_const(gdev->label);
12451245
err_free_descs:

0 commit comments

Comments
 (0)