diff mbox series

[v3,22/24] gpio: protect the pointer to gpio_chip in gpio_device with SRCU

Message ID 20240208095920.8035-23-brgl@bgdev.pl
State New
Headers show
Series gpio: rework locking and object life-time control | expand

Commit Message

Bartosz Golaszewski Feb. 8, 2024, 9:59 a.m. UTC
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Ensure we cannot crash if the GPIO device gets unregistered (and the
chip pointer set to NULL) during any of the API calls.

To that end: wait for all users of gdev->chip to exit their read-only
SRCU critical sections in gpiochip_remove().

For brevity: add a guard class which can be instantiated at the top of
every function requiring read-only access to the chip pointer and use it
in all API calls taking a GPIO descriptor as argument. In places where
we only deal with the GPIO device - use regular guard() helpers and
rcu_dereference() for chip access. Do the same in API calls taking a
const pointer to gpio_desc.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib-cdev.c  |  63 +++++----
 drivers/gpio/gpiolib-sysfs.c |  57 +++++---
 drivers/gpio/gpiolib.c       | 257 +++++++++++++++++++++++------------
 drivers/gpio/gpiolib.h       |  22 ++-
 4 files changed, 271 insertions(+), 128 deletions(-)

Comments

kernel test robot Feb. 12, 2024, 3:09 p.m. UTC | #1
Hello,

kernel test robot noticed "WARNING:suspicious_RCU_usage" on:

commit: c21131f83abc1f7227e7a6d5311e1df68bfa44e0 ("[PATCH v3 22/24] gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpio-protect-the-list-of-GPIO-devices-with-SRCU/20240208-180822
base: https://git.kernel.org/cgit/linux/kernel/git/brgl/linux.git gpio/for-next
patch link: https://lore.kernel.org/all/20240208095920.8035-23-brgl@bgdev.pl/
patch subject: [PATCH v3 22/24] gpio: protect the pointer to gpio_chip in gpio_device with SRCU

in testcase: boot

compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+-----------------------------------------------------------------+------------+------------+
|                                                                 | a3dfc11062 | c21131f83a |
+-----------------------------------------------------------------+------------+------------+
| drivers/gpio/gpiolib.c:#suspicious_rcu_dereference_check()usage | 0          | 8          |
| drivers/gpio/gpiolib.h:#suspicious_rcu_dereference_check()usage | 0          | 8          |
+-----------------------------------------------------------------+------------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.com



[   76.432519][    T1] gpiochip_find_base_unlocked: found new base at 512
[   76.434591][    T1]
[   76.435240][    T1] =============================
[   76.436545][    T1] WARNING: suspicious RCU usage
[   76.437813][    T1] 6.8.0-rc1-00050-gc21131f83abc #1 Tainted: G                 N
[   76.439873][    T1] -----------------------------
[   76.441158][    T1] drivers/gpio/gpiolib.c:219 suspicious rcu_dereference_check() usage!
[   76.443364][    T1]
[   76.443364][    T1] other info that might help us debug this:
[   76.443364][    T1]
[   76.446059][    T1]
[   76.446059][    T1] rcu_scheduler_active = 2, debug_locks = 1
[   76.448217][    T1] 1 lock held by swapper/1:
[ 76.449412][ T1] #0: ffff88816954f0f0 (&dev->mutex){....}-{3:3}, at: __driver_attach (drivers/base/dd.c:1216) 
[   76.451938][    T1]
[   76.451938][    T1] stack backtrace:
[   76.453486][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                 N 6.8.0-rc1-00050-gc21131f83abc #1
[   76.456114][    T1] Call Trace:
[   76.456936][    T1]  <TASK>
[ 76.457682][ T1] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1)) 
[ 76.458833][ T1] lockdep_rcu_suspicious (include/linux/context_tracking.h:153 kernel/locking/lockdep.c:6713) 
[ 76.460205][ T1] gpiod_to_chip (drivers/gpio/gpiolib.c:219 (discriminator 9)) 
[ 76.461346][ T1] gpiod_hog (drivers/gpio/gpiolib.h:243 drivers/gpio/gpiolib.c:4502) 
[ 76.462400][ T1] ? of_find_property (drivers/of/base.c:223) 
[ 76.463671][ T1] of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:799) 
[ 76.464933][ T1] ? __pfx_of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:785) 
[ 76.466378][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360) 
[ 76.467894][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423) 
[ 76.469220][ T1] ? _raw_spin_unlock_irqrestore (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:152 kernel/locking/spinlock.c:194) 
[ 76.470786][ T1] of_gpiochip_add (drivers/gpio/gpiolib-of.c:828 drivers/gpio/gpiolib-of.c:1143) 
[ 76.472060][ T1] ? fwnode_property_read_int_array (drivers/base/property.c:268 (discriminator 5)) 
[ 76.473692][ T1] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:989) 
[ 76.475271][ T1] ? kasan_save_track (arch/x86/include/asm/current.h:42 mm/kasan/common.c:60 mm/kasan/common.c:70) 
[ 76.476567][ T1] unittest_gpio_probe (drivers/of/unittest.c:1886) 
[ 76.477928][ T1] platform_probe (drivers/base/platform.c:1404) 
[ 76.479162][ T1] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658) 
[ 76.480403][ T1] __driver_probe_device (drivers/base/dd.c:800) 
[ 76.481791][ T1] driver_probe_device (drivers/base/dd.c:830) 
[ 76.483097][ T1] __driver_attach (drivers/base/dd.c:1217) 
[ 76.484388][ T1] ? __pfx___driver_attach (drivers/base/dd.c:1157) 
[ 76.485805][ T1] bus_for_each_dev (drivers/base/bus.c:367) 
[ 76.487037][ T1] ? lockdep_init_map_type (kernel/locking/lockdep.c:4892) 
[ 76.488477][ T1] ? __pfx_bus_for_each_dev (drivers/base/bus.c:356) 
[ 76.489897][ T1] ? bus_add_driver (drivers/base/bus.c:672) 
[ 76.491195][ T1] bus_add_driver (drivers/base/bus.c:674) 
[ 76.492463][ T1] driver_register (drivers/base/driver.c:246) 
[ 76.493723][ T1] of_unittest_overlay_gpio (drivers/of/unittest.c:1969 (discriminator 4)) 
[ 76.495167][ T1] of_unittest_overlay (drivers/of/unittest.c:2189 drivers/of/unittest.c:3217) 
[ 76.496478][ T1] ? __pfx_of_unittest_overlay (drivers/of/unittest.c:3155) 
[ 76.497886][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360) 
[ 76.499410][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423) 
[ 76.500744][ T1] of_unittest (drivers/of/unittest.c:4129) 
[ 76.501862][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080) 
[ 76.503098][ T1] ? add_device_randomness (drivers/char/random.c:918) 
[ 76.504492][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080) 
[ 76.505807][ T1] do_one_initcall (init/main.c:1236) 
[ 76.507054][ T1] ? __pfx_do_one_initcall (init/main.c:1227) 
[ 76.508517][ T1] do_initcalls (init/main.c:1297 init/main.c:1314) 
[ 76.509731][ T1] kernel_init_freeable (init/main.c:1555) 
[ 76.511106][ T1] ? __pfx_kernel_init (init/main.c:1433) 
[ 76.512435][ T1] kernel_init (init/main.c:1443) 
[ 76.513566][ T1] ? _raw_spin_unlock_irq (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:160 kernel/locking/spinlock.c:202) 
[ 76.514947][ T1] ret_from_fork (arch/x86/kernel/process.c:153) 
[ 76.516125][ T1] ? __pfx_kernel_init (init/main.c:1433) 
[ 76.517440][ T1] ret_from_fork_asm (arch/x86/entry/entry_64.S:250) 
[   76.518731][    T1]  </TASK>
[   76.519758][    T1]
[   76.520477][    T1] =============================
[   76.521774][    T1] WARNING: suspicious RCU usage
[   76.523076][    T1] 6.8.0-rc1-00050-gc21131f83abc #1 Tainted: G                 N
[   76.525108][    T1] -----------------------------
[   76.526429][    T1] drivers/gpio/gpiolib.h:210 suspicious rcu_dereference_check() usage!
[   76.528621][    T1]
[   76.528621][    T1] other info that might help us debug this:
[   76.528621][    T1]
[   76.531350][    T1]
[   76.531350][    T1] rcu_scheduler_active = 2, debug_locks = 1
[   76.533414][    T1] 2 locks held by swapper/1:
[ 76.534616][ T1] #0: ffff88816954f0f0 (&dev->mutex){....}-{3:3}, at: __driver_attach (drivers/base/dd.c:1216) 
[ 76.537073][ T1] #1: ffff888163afb6d0 (&gdev->srcu){.+.+}-{0:0}, at: gpiod_request_commit (include/linux/srcu.h:116 include/linux/srcu.h:215 drivers/gpio/gpiolib.h:202 drivers/gpio/gpiolib.c:2243) 
[   76.539703][    T1]
[   76.539703][    T1] stack backtrace:
[   76.541276][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                 N 6.8.0-rc1-00050-gc21131f83abc #1
[   76.543890][    T1] Call Trace:
[   76.544767][    T1]  <TASK>
[ 76.545549][ T1] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1)) 
[ 76.546740][ T1] lockdep_rcu_suspicious (include/linux/context_tracking.h:153 kernel/locking/lockdep.c:6713) 
[ 76.548196][ T1] gpiod_request_commit (drivers/gpio/gpiolib.h:202 drivers/gpio/gpiolib.c:2243) 
[ 76.549584][ T1] ? dump_stack_lvl (lib/dump_stack.c:108) 
[ 76.550829][ T1] gpiochip_request_own_desc (drivers/gpio/gpiolib.c:2454) 
[ 76.552354][ T1] gpiod_hog (drivers/gpio/gpiolib.c:4504) 
[ 76.553556][ T1] of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:799) 
[ 76.554948][ T1] ? __pfx_of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:785) 
[ 76.556536][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360) 
[ 76.558176][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423) 
[ 76.559633][ T1] ? _raw_spin_unlock_irqrestore (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:152 kernel/locking/spinlock.c:194) 
[ 76.561252][ T1] of_gpiochip_add (drivers/gpio/gpiolib-of.c:828 drivers/gpio/gpiolib-of.c:1143) 
[ 76.562584][ T1] ? fwnode_property_read_int_array (drivers/base/property.c:268 (discriminator 5)) 
[ 76.564302][ T1] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:989) 
[ 76.565974][ T1] ? kasan_save_track (arch/x86/include/asm/current.h:42 mm/kasan/common.c:60 mm/kasan/common.c:70) 
[ 76.567328][ T1] unittest_gpio_probe (drivers/of/unittest.c:1886) 
[ 76.568780][ T1] platform_probe (drivers/base/platform.c:1404) 
[ 76.570065][ T1] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658) 
[ 76.571309][ T1] __driver_probe_device (drivers/base/dd.c:800) 
[ 76.572801][ T1] driver_probe_device (drivers/base/dd.c:830) 
[ 76.574175][ T1] __driver_attach (drivers/base/dd.c:1217) 
[ 76.575513][ T1] ? __pfx___driver_attach (drivers/base/dd.c:1157) 
[ 76.576994][ T1] bus_for_each_dev (drivers/base/bus.c:367) 
[ 76.578331][ T1] ? lockdep_init_map_type (kernel/locking/lockdep.c:4892) 
[ 76.579877][ T1] ? __pfx_bus_for_each_dev (drivers/base/bus.c:356) 
[ 76.581368][ T1] ? bus_add_driver (drivers/base/bus.c:672) 
[ 76.582799][ T1] bus_add_driver (drivers/base/bus.c:674) 
[ 76.584128][ T1] driver_register (drivers/base/driver.c:246) 
[ 76.585452][ T1] of_unittest_overlay_gpio (drivers/of/unittest.c:1969 (discriminator 4)) 
[ 76.586969][ T1] of_unittest_overlay (drivers/of/unittest.c:2189 drivers/of/unittest.c:3217) 
[ 76.588400][ T1] ? __pfx_of_unittest_overlay (drivers/of/unittest.c:3155) 
[ 76.589958][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360) 
[ 76.591624][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423) 
[ 76.593071][ T1] of_unittest (drivers/of/unittest.c:4129) 
[ 76.594323][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080) 
[ 76.595700][ T1] ? add_device_randomness (drivers/char/random.c:918) 
[ 76.597164][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080) 
[ 76.598482][ T1] do_one_initcall (init/main.c:1236) 
[ 76.599728][ T1] ? __pfx_do_one_initcall (init/main.c:1227) 
[ 76.601163][ T1] do_initcalls (init/main.c:1297 init/main.c:1314) 
[ 76.602389][ T1] kernel_init_freeable (init/main.c:1555) 
[ 76.603810][ T1] ? __pfx_kernel_init (init/main.c:1433) 
[ 76.605101][ T1] kernel_init (init/main.c:1443) 
[ 76.606235][ T1] ? _raw_spin_unlock_irq (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:160 kernel/locking/spinlock.c:202) 
[ 76.607627][ T1] ret_from_fork (arch/x86/kernel/process.c:153) 
[ 76.608797][ T1] ? __pfx_kernel_init (init/main.c:1433) 
[ 76.610112][ T1] ret_from_fork_asm (arch/x86/entry/entry_64.S:250) 
[   76.611412][    T1]  </TASK>
[   76.612591][    T1] general protection fault, probably for non-canonical address 0xdffffc000000002f: 0000 [#1] PREEMPT KASAN PTI
[   76.615654][    T1] KASAN: null-ptr-deref in range [0x0000000000000178-0x000000000000017f]
[   76.617847][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                 N 6.8.0-rc1-00050-gc21131f83abc #1
[ 76.620463][ T1] RIP: 0010:check_init_srcu_struct (kernel/rcu/srcutree.c:408) 
[ 76.622072][ T1] Code: 53 48 89 fb 80 3c 02 00 0f 85 fe 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 38 48 8d bd 78 01 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 ce 00 00 00 48 8b 85 78 01 00 00 a8 03 75 0b 5b
All code
========
   0:	53                   	push   %rbx
   1:	48 89 fb             	mov    %rdi,%rbx
   4:	80 3c 02 00          	cmpb   $0x0,(%rdx,%rax,1)
   8:	0f 85 fe 00 00 00    	jne    0x10c
   e:	48 b8 00 00 00 00 00 	movabs $0xdffffc0000000000,%rax
  15:	fc ff df 
  18:	48 8b 6b 38          	mov    0x38(%rbx),%rbp
  1c:	48 8d bd 78 01 00 00 	lea    0x178(%rbp),%rdi
  23:	48 89 fa             	mov    %rdi,%rdx
  26:	48 c1 ea 03          	shr    $0x3,%rdx
  2a:*	80 3c 02 00          	cmpb   $0x0,(%rdx,%rax,1)		<-- trapping instruction
  2e:	0f 85 ce 00 00 00    	jne    0x102
  34:	48 8b 85 78 01 00 00 	mov    0x178(%rbp),%rax
  3b:	a8 03                	test   $0x3,%al
  3d:	75 0b                	jne    0x4a
  3f:	5b                   	pop    %rbx

Code starting with the faulting instruction
===========================================
   0:	80 3c 02 00          	cmpb   $0x0,(%rdx,%rax,1)
   4:	0f 85 ce 00 00 00    	jne    0xd8
   a:	48 8b 85 78 01 00 00 	mov    0x178(%rbp),%rax
  11:	a8 03                	test   $0x3,%al
  13:	75 0b                	jne    0x20
  15:	5b                   	pop    %rbx
[   76.627183][    T1] RSP: 0018:ffff888103a6f718 EFLAGS: 00010202
[   76.628803][    T1] RAX: dffffc0000000000 RBX: ffff88810ee660f8 RCX: 0000000000000000
[   76.630879][    T1] RDX: 000000000000002f RSI: ffff88816976b000 RDI: 0000000000000178
[   76.632960][    T1] RBP: 0000000000000000 R08: 692d422d656e696c R09: 007475706e692d42
[   76.635045][    T1] R10: ffff888103a6f750 R11: ffffffff810b3aef R12: ffff88810ee66130
[   76.641151][    T1] R13: ffff888163afb6c0 R14: 0000000000000000 R15: ffff888163afb6d0
[   76.643224][    T1] FS:  0000000000000000(0000) GS:ffffffff84cd1000(0000) knlGS:0000000000000000
[   76.645563][    T1] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   76.647278][    T1] CR2: 00007fab8f4456f4 CR3: 0000000004cac000 CR4: 00000000000406b0
[   76.649355][    T1] Call Trace:
[   76.650221][    T1]  <TASK>
[ 76.651020][ T1] ? die_addr (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:460) 
[ 76.652132][ T1] ? exc_general_protection (arch/x86/kernel/traps.c:701 arch/x86/kernel/traps.c:643) 
[ 76.653604][ T1] ? asm_exc_general_protection (arch/x86/include/asm/idtentry.h:564) 
[ 76.655133][ T1] ? ret_from_fork (arch/x86/kernel/process.c:153) 
[ 76.656355][ T1] ? check_init_srcu_struct (kernel/rcu/srcutree.c:408) 
[ 76.657792][ T1] synchronize_srcu (kernel/rcu/srcutree.c:1167 kernel/rcu/srcutree.c:1458) 
[ 76.659048][ T1] gpiod_request_commit (drivers/gpio/gpiolib.c:127 drivers/gpio/gpiolib.c:2273) 
[ 76.660430][ T1] gpiochip_request_own_desc (drivers/gpio/gpiolib.c:2454) 
[ 76.661898][ T1] gpiod_hog (drivers/gpio/gpiolib.c:4504) 
[ 76.663000][ T1] of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:799) 
[ 76.664334][ T1] ? __pfx_of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:785) 
[ 76.665830][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360) 
[ 76.667387][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423) 
[ 76.668763][ T1] ? _raw_spin_unlock_irqrestore (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:152 kernel/locking/spinlock.c:194) 
[ 76.670279][ T1] of_gpiochip_add (drivers/gpio/gpiolib-of.c:828 drivers/gpio/gpiolib-of.c:1143) 
[ 76.671550][ T1] ? fwnode_property_read_int_array (drivers/base/property.c:268 (discriminator 5)) 
[ 76.673171][ T1] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:989) 
[ 76.674714][ T1] ? kasan_save_track (arch/x86/include/asm/current.h:42 mm/kasan/common.c:60 mm/kasan/common.c:70) 
[ 76.676002][ T1] unittest_gpio_probe (drivers/of/unittest.c:1886) 
[ 76.677340][ T1] platform_probe (drivers/base/platform.c:1404) 
[ 76.678544][ T1] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658) 
[ 76.679745][ T1] __driver_probe_device (drivers/base/dd.c:800) 
[ 76.681126][ T1] driver_probe_device (drivers/base/dd.c:830) 
[ 76.682411][ T1] __driver_attach (drivers/base/dd.c:1217) 
[ 76.683679][ T1] ? __pfx___driver_attach (drivers/base/dd.c:1157) 
[ 76.685074][ T1] bus_for_each_dev (drivers/base/bus.c:367) 
[ 76.686340][ T1] ? lockdep_init_map_type (kernel/locking/lockdep.c:4892) 
[ 76.687774][ T1] ? __pfx_bus_for_each_dev (drivers/base/bus.c:356) 
[ 76.689176][ T1] ? bus_add_driver (drivers/base/bus.c:672) 
[ 76.690452][ T1] bus_add_driver (drivers/base/bus.c:674) 
[ 76.691691][ T1] driver_register (drivers/base/driver.c:246) 
[ 76.692949][ T1] of_unittest_overlay_gpio (drivers/of/unittest.c:1969 (discriminator 4)) 
[ 76.694377][ T1] of_unittest_overlay (drivers/of/unittest.c:2189 drivers/of/unittest.c:3217) 
[ 76.695724][ T1] ? __pfx_of_unittest_overlay (drivers/of/unittest.c:3155) 
[ 76.697198][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360) 
[ 76.698764][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423) 
[ 76.700151][ T1] of_unittest (drivers/of/unittest.c:4129) 
[ 76.701323][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080) 
[ 76.702622][ T1] ? add_device_randomness (drivers/char/random.c:918) 
[ 76.704050][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080) 
[ 76.705347][ T1] do_one_initcall (init/main.c:1236) 
[ 76.706572][ T1] ? __pfx_do_one_initcall (init/main.c:1227) 
[ 76.708227][ T1] do_initcalls (init/main.c:1297 init/main.c:1314) 


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240212/202402122234.d85cca9b-lkp@intel.com
Bartosz Golaszewski Feb. 12, 2024, 4:56 p.m. UTC | #2
On Mon, Feb 12, 2024 at 4:11 PM kernel test robot <oliver.sang@intel.com> wrote:
>
>
>
> Hello,
>
> kernel test robot noticed "WARNING:suspicious_RCU_usage" on:
>
> commit: c21131f83abc1f7227e7a6d5311e1df68bfa44e0 ("[PATCH v3 22/24] gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
> url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpio-protect-the-list-of-GPIO-devices-with-SRCU/20240208-180822
> base: https://git.kernel.org/cgit/linux/kernel/git/brgl/linux.git gpio/for-next
> patch link: https://lore.kernel.org/all/20240208095920.8035-23-brgl@bgdev.pl/
> patch subject: [PATCH v3 22/24] gpio: protect the pointer to gpio_chip in gpio_device with SRCU
>
> in testcase: boot
>
> compiler: gcc-12
> test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
>
> (please refer to attached dmesg/kmsg for entire log/backtrace)
>
>
> +-----------------------------------------------------------------+------------+------------+
> |                                                                 | a3dfc11062 | c21131f83a |
> +-----------------------------------------------------------------+------------+------------+
> | drivers/gpio/gpiolib.c:#suspicious_rcu_dereference_check()usage | 0          | 8          |
> | drivers/gpio/gpiolib.h:#suspicious_rcu_dereference_check()usage | 0          | 8          |
> +-----------------------------------------------------------------+------------+------------+
>
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.com
>
>
>
> [   76.432519][    T1] gpiochip_find_base_unlocked: found new base at 512
> [   76.434591][    T1]
> [   76.435240][    T1] =============================
> [   76.436545][    T1] WARNING: suspicious RCU usage
> [   76.437813][    T1] 6.8.0-rc1-00050-gc21131f83abc #1 Tainted: G                 N
> [   76.439873][    T1] -----------------------------
> [   76.441158][    T1] drivers/gpio/gpiolib.c:219 suspicious rcu_dereference_check() usage!
> [   76.443364][    T1]
> [   76.443364][    T1] other info that might help us debug this:
> [   76.443364][    T1]
> [   76.446059][    T1]
> [   76.446059][    T1] rcu_scheduler_active = 2, debug_locks = 1
> [   76.448217][    T1] 1 lock held by swapper/1:
> [ 76.449412][ T1] #0: ffff88816954f0f0 (&dev->mutex){....}-{3:3}, at: __driver_attach (drivers/base/dd.c:1216)
> [   76.451938][    T1]
> [   76.451938][    T1] stack backtrace:
> [   76.453486][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                 N 6.8.0-rc1-00050-gc21131f83abc #1
> [   76.456114][    T1] Call Trace:
> [   76.456936][    T1]  <TASK>
> [ 76.457682][ T1] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
> [ 76.458833][ T1] lockdep_rcu_suspicious (include/linux/context_tracking.h:153 kernel/locking/lockdep.c:6713)
> [ 76.460205][ T1] gpiod_to_chip (drivers/gpio/gpiolib.c:219 (discriminator 9))
> [ 76.461346][ T1] gpiod_hog (drivers/gpio/gpiolib.h:243 drivers/gpio/gpiolib.c:4502)

Ah, gpiod_hog() is not taking the SRCU read lock as it should. I'll send a fix.

Bart

[snip]
Bartosz Golaszewski Feb. 12, 2024, 9:20 p.m. UTC | #3
On Mon, Feb 12, 2024 at 4:11 PM kernel test robot <oliver.sang@intel.com> wrote:
>
>
>
> Hello,
>
> kernel test robot noticed "WARNING:suspicious_RCU_usage" on:
>
> commit: c21131f83abc1f7227e7a6d5311e1df68bfa44e0 ("[PATCH v3 22/24] gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
> url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpio-protect-the-list-of-GPIO-devices-with-SRCU/20240208-180822
> base: https://git.kernel.org/cgit/linux/kernel/git/brgl/linux.git gpio/for-next
> patch link: https://lore.kernel.org/all/20240208095920.8035-23-brgl@bgdev.pl/
> patch subject: [PATCH v3 22/24] gpio: protect the pointer to gpio_chip in gpio_device with SRCU
>
> in testcase: boot
>
> compiler: gcc-12
> test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
>
> (please refer to attached dmesg/kmsg for entire log/backtrace)
>
>
> +-----------------------------------------------------------------+------------+------------+
> |                                                                 | a3dfc11062 | c21131f83a |
> +-----------------------------------------------------------------+------------+------------+
> | drivers/gpio/gpiolib.c:#suspicious_rcu_dereference_check()usage | 0          | 8          |
> | drivers/gpio/gpiolib.h:#suspicious_rcu_dereference_check()usage | 0          | 8          |
> +-----------------------------------------------------------------+------------+------------+
>
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.com
>
>
>
> [   76.432519][    T1] gpiochip_find_base_unlocked: found new base at 512
> [   76.434591][    T1]
> [   76.435240][    T1] =============================
> [   76.436545][    T1] WARNING: suspicious RCU usage
> [   76.437813][    T1] 6.8.0-rc1-00050-gc21131f83abc #1 Tainted: G                 N
> [   76.439873][    T1] -----------------------------
> [   76.441158][    T1] drivers/gpio/gpiolib.c:219 suspicious rcu_dereference_check() usage!
> [   76.443364][    T1]
> [   76.443364][    T1] other info that might help us debug this:
> [   76.443364][    T1]
> [   76.446059][    T1]
> [   76.446059][    T1] rcu_scheduler_active = 2, debug_locks = 1
> [   76.448217][    T1] 1 lock held by swapper/1:
> [ 76.449412][ T1] #0: ffff88816954f0f0 (&dev->mutex){....}-{3:3}, at: __driver_attach (drivers/base/dd.c:1216)
> [   76.451938][    T1]
> [   76.451938][    T1] stack backtrace:
> [   76.453486][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                 N 6.8.0-rc1-00050-gc21131f83abc #1
> [   76.456114][    T1] Call Trace:
> [   76.456936][    T1]  <TASK>
> [ 76.457682][ T1] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
> [ 76.458833][ T1] lockdep_rcu_suspicious (include/linux/context_tracking.h:153 kernel/locking/lockdep.c:6713)
> [ 76.460205][ T1] gpiod_to_chip (drivers/gpio/gpiolib.c:219 (discriminator 9))
> [ 76.461346][ T1] gpiod_hog (drivers/gpio/gpiolib.h:243 drivers/gpio/gpiolib.c:4502)
> [ 76.462400][ T1] ? of_find_property (drivers/of/base.c:223)
> [ 76.463671][ T1] of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:799)
> [ 76.464933][ T1] ? __pfx_of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:785)
> [ 76.466378][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360)
> [ 76.467894][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423)
> [ 76.469220][ T1] ? _raw_spin_unlock_irqrestore (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:152 kernel/locking/spinlock.c:194)
> [ 76.470786][ T1] of_gpiochip_add (drivers/gpio/gpiolib-of.c:828 drivers/gpio/gpiolib-of.c:1143)
> [ 76.472060][ T1] ? fwnode_property_read_int_array (drivers/base/property.c:268 (discriminator 5))
> [ 76.473692][ T1] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:989)
> [ 76.475271][ T1] ? kasan_save_track (arch/x86/include/asm/current.h:42 mm/kasan/common.c:60 mm/kasan/common.c:70)
> [ 76.476567][ T1] unittest_gpio_probe (drivers/of/unittest.c:1886)
> [ 76.477928][ T1] platform_probe (drivers/base/platform.c:1404)
> [ 76.479162][ T1] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658)
> [ 76.480403][ T1] __driver_probe_device (drivers/base/dd.c:800)
> [ 76.481791][ T1] driver_probe_device (drivers/base/dd.c:830)
> [ 76.483097][ T1] __driver_attach (drivers/base/dd.c:1217)
> [ 76.484388][ T1] ? __pfx___driver_attach (drivers/base/dd.c:1157)
> [ 76.485805][ T1] bus_for_each_dev (drivers/base/bus.c:367)
> [ 76.487037][ T1] ? lockdep_init_map_type (kernel/locking/lockdep.c:4892)
> [ 76.488477][ T1] ? __pfx_bus_for_each_dev (drivers/base/bus.c:356)
> [ 76.489897][ T1] ? bus_add_driver (drivers/base/bus.c:672)
> [ 76.491195][ T1] bus_add_driver (drivers/base/bus.c:674)
> [ 76.492463][ T1] driver_register (drivers/base/driver.c:246)
> [ 76.493723][ T1] of_unittest_overlay_gpio (drivers/of/unittest.c:1969 (discriminator 4))
> [ 76.495167][ T1] of_unittest_overlay (drivers/of/unittest.c:2189 drivers/of/unittest.c:3217)
> [ 76.496478][ T1] ? __pfx_of_unittest_overlay (drivers/of/unittest.c:3155)
> [ 76.497886][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360)
> [ 76.499410][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423)
> [ 76.500744][ T1] of_unittest (drivers/of/unittest.c:4129)
> [ 76.501862][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080)
> [ 76.503098][ T1] ? add_device_randomness (drivers/char/random.c:918)
> [ 76.504492][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080)
> [ 76.505807][ T1] do_one_initcall (init/main.c:1236)
> [ 76.507054][ T1] ? __pfx_do_one_initcall (init/main.c:1227)
> [ 76.508517][ T1] do_initcalls (init/main.c:1297 init/main.c:1314)
> [ 76.509731][ T1] kernel_init_freeable (init/main.c:1555)
> [ 76.511106][ T1] ? __pfx_kernel_init (init/main.c:1433)
> [ 76.512435][ T1] kernel_init (init/main.c:1443)
> [ 76.513566][ T1] ? _raw_spin_unlock_irq (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:160 kernel/locking/spinlock.c:202)
> [ 76.514947][ T1] ret_from_fork (arch/x86/kernel/process.c:153)
> [ 76.516125][ T1] ? __pfx_kernel_init (init/main.c:1433)
> [ 76.517440][ T1] ret_from_fork_asm (arch/x86/entry/entry_64.S:250)
> [   76.518731][    T1]  </TASK>
> [   76.519758][    T1]
> [   76.520477][    T1] =============================
> [   76.521774][    T1] WARNING: suspicious RCU usage
> [   76.523076][    T1] 6.8.0-rc1-00050-gc21131f83abc #1 Tainted: G                 N
> [   76.525108][    T1] -----------------------------
> [   76.526429][    T1] drivers/gpio/gpiolib.h:210 suspicious rcu_dereference_check() usage!
> [   76.528621][    T1]
> [   76.528621][    T1] other info that might help us debug this:
> [   76.528621][    T1]
> [   76.531350][    T1]
> [   76.531350][    T1] rcu_scheduler_active = 2, debug_locks = 1
> [   76.533414][    T1] 2 locks held by swapper/1:
> [ 76.534616][ T1] #0: ffff88816954f0f0 (&dev->mutex){....}-{3:3}, at: __driver_attach (drivers/base/dd.c:1216)
> [ 76.537073][ T1] #1: ffff888163afb6d0 (&gdev->srcu){.+.+}-{0:0}, at: gpiod_request_commit (include/linux/srcu.h:116 include/linux/srcu.h:215 drivers/gpio/gpiolib.h:202 drivers/gpio/gpiolib.c:2243)
> [   76.539703][    T1]
> [   76.539703][    T1] stack backtrace:
> [   76.541276][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                 N 6.8.0-rc1-00050-gc21131f83abc #1
> [   76.543890][    T1] Call Trace:
> [   76.544767][    T1]  <TASK>
> [ 76.545549][ T1] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
> [ 76.546740][ T1] lockdep_rcu_suspicious (include/linux/context_tracking.h:153 kernel/locking/lockdep.c:6713)
> [ 76.548196][ T1] gpiod_request_commit (drivers/gpio/gpiolib.h:202 drivers/gpio/gpiolib.c:2243)
> [ 76.549584][ T1] ? dump_stack_lvl (lib/dump_stack.c:108)
> [ 76.550829][ T1] gpiochip_request_own_desc (drivers/gpio/gpiolib.c:2454)
> [ 76.552354][ T1] gpiod_hog (drivers/gpio/gpiolib.c:4504)
> [ 76.553556][ T1] of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:799)
> [ 76.554948][ T1] ? __pfx_of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:785)
> [ 76.556536][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360)
> [ 76.558176][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423)
> [ 76.559633][ T1] ? _raw_spin_unlock_irqrestore (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:152 kernel/locking/spinlock.c:194)
> [ 76.561252][ T1] of_gpiochip_add (drivers/gpio/gpiolib-of.c:828 drivers/gpio/gpiolib-of.c:1143)
> [ 76.562584][ T1] ? fwnode_property_read_int_array (drivers/base/property.c:268 (discriminator 5))
> [ 76.564302][ T1] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:989)
> [ 76.565974][ T1] ? kasan_save_track (arch/x86/include/asm/current.h:42 mm/kasan/common.c:60 mm/kasan/common.c:70)
> [ 76.567328][ T1] unittest_gpio_probe (drivers/of/unittest.c:1886)
> [ 76.568780][ T1] platform_probe (drivers/base/platform.c:1404)
> [ 76.570065][ T1] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658)
> [ 76.571309][ T1] __driver_probe_device (drivers/base/dd.c:800)
> [ 76.572801][ T1] driver_probe_device (drivers/base/dd.c:830)
> [ 76.574175][ T1] __driver_attach (drivers/base/dd.c:1217)
> [ 76.575513][ T1] ? __pfx___driver_attach (drivers/base/dd.c:1157)
> [ 76.576994][ T1] bus_for_each_dev (drivers/base/bus.c:367)
> [ 76.578331][ T1] ? lockdep_init_map_type (kernel/locking/lockdep.c:4892)
> [ 76.579877][ T1] ? __pfx_bus_for_each_dev (drivers/base/bus.c:356)
> [ 76.581368][ T1] ? bus_add_driver (drivers/base/bus.c:672)
> [ 76.582799][ T1] bus_add_driver (drivers/base/bus.c:674)
> [ 76.584128][ T1] driver_register (drivers/base/driver.c:246)
> [ 76.585452][ T1] of_unittest_overlay_gpio (drivers/of/unittest.c:1969 (discriminator 4))
> [ 76.586969][ T1] of_unittest_overlay (drivers/of/unittest.c:2189 drivers/of/unittest.c:3217)
> [ 76.588400][ T1] ? __pfx_of_unittest_overlay (drivers/of/unittest.c:3155)
> [ 76.589958][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360)
> [ 76.591624][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423)
> [ 76.593071][ T1] of_unittest (drivers/of/unittest.c:4129)
> [ 76.594323][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080)
> [ 76.595700][ T1] ? add_device_randomness (drivers/char/random.c:918)
> [ 76.597164][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080)
> [ 76.598482][ T1] do_one_initcall (init/main.c:1236)
> [ 76.599728][ T1] ? __pfx_do_one_initcall (init/main.c:1227)
> [ 76.601163][ T1] do_initcalls (init/main.c:1297 init/main.c:1314)
> [ 76.602389][ T1] kernel_init_freeable (init/main.c:1555)
> [ 76.603810][ T1] ? __pfx_kernel_init (init/main.c:1433)
> [ 76.605101][ T1] kernel_init (init/main.c:1443)
> [ 76.606235][ T1] ? _raw_spin_unlock_irq (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:160 kernel/locking/spinlock.c:202)
> [ 76.607627][ T1] ret_from_fork (arch/x86/kernel/process.c:153)
> [ 76.608797][ T1] ? __pfx_kernel_init (init/main.c:1433)
> [ 76.610112][ T1] ret_from_fork_asm (arch/x86/entry/entry_64.S:250)
> [   76.611412][    T1]  </TASK>
> [   76.612591][    T1] general protection fault, probably for non-canonical address 0xdffffc000000002f: 0000 [#1] PREEMPT KASAN PTI
> [   76.615654][    T1] KASAN: null-ptr-deref in range [0x0000000000000178-0x000000000000017f]
> [   76.617847][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                 N 6.8.0-rc1-00050-gc21131f83abc #1
> [ 76.620463][ T1] RIP: 0010:check_init_srcu_struct (kernel/rcu/srcutree.c:408)
> [ 76.622072][ T1] Code: 53 48 89 fb 80 3c 02 00 0f 85 fe 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 38 48 8d bd 78 01 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 ce 00 00 00 48 8b 85 78 01 00 00 a8 03 75 0b 5b
> All code
> ========
>    0:   53                      push   %rbx
>    1:   48 89 fb                mov    %rdi,%rbx
>    4:   80 3c 02 00             cmpb   $0x0,(%rdx,%rax,1)
>    8:   0f 85 fe 00 00 00       jne    0x10c
>    e:   48 b8 00 00 00 00 00    movabs $0xdffffc0000000000,%rax
>   15:   fc ff df
>   18:   48 8b 6b 38             mov    0x38(%rbx),%rbp
>   1c:   48 8d bd 78 01 00 00    lea    0x178(%rbp),%rdi
>   23:   48 89 fa                mov    %rdi,%rdx
>   26:   48 c1 ea 03             shr    $0x3,%rdx
>   2a:*  80 3c 02 00             cmpb   $0x0,(%rdx,%rax,1)               <-- trapping instruction
>   2e:   0f 85 ce 00 00 00       jne    0x102
>   34:   48 8b 85 78 01 00 00    mov    0x178(%rbp),%rax
>   3b:   a8 03                   test   $0x3,%al
>   3d:   75 0b                   jne    0x4a
>   3f:   5b                      pop    %rbx
>
> Code starting with the faulting instruction
> ===========================================
>    0:   80 3c 02 00             cmpb   $0x0,(%rdx,%rax,1)
>    4:   0f 85 ce 00 00 00       jne    0xd8
>    a:   48 8b 85 78 01 00 00    mov    0x178(%rbp),%rax
>   11:   a8 03                   test   $0x3,%al
>   13:   75 0b                   jne    0x20
>   15:   5b                      pop    %rbx
> [   76.627183][    T1] RSP: 0018:ffff888103a6f718 EFLAGS: 00010202
> [   76.628803][    T1] RAX: dffffc0000000000 RBX: ffff88810ee660f8 RCX: 0000000000000000
> [   76.630879][    T1] RDX: 000000000000002f RSI: ffff88816976b000 RDI: 0000000000000178
> [   76.632960][    T1] RBP: 0000000000000000 R08: 692d422d656e696c R09: 007475706e692d42
> [   76.635045][    T1] R10: ffff888103a6f750 R11: ffffffff810b3aef R12: ffff88810ee66130
> [   76.641151][    T1] R13: ffff888163afb6c0 R14: 0000000000000000 R15: ffff888163afb6d0
> [   76.643224][    T1] FS:  0000000000000000(0000) GS:ffffffff84cd1000(0000) knlGS:0000000000000000
> [   76.645563][    T1] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   76.647278][    T1] CR2: 00007fab8f4456f4 CR3: 0000000004cac000 CR4: 00000000000406b0
> [   76.649355][    T1] Call Trace:
> [   76.650221][    T1]  <TASK>
> [ 76.651020][ T1] ? die_addr (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:460)
> [ 76.652132][ T1] ? exc_general_protection (arch/x86/kernel/traps.c:701 arch/x86/kernel/traps.c:643)
> [ 76.653604][ T1] ? asm_exc_general_protection (arch/x86/include/asm/idtentry.h:564)
> [ 76.655133][ T1] ? ret_from_fork (arch/x86/kernel/process.c:153)
> [ 76.656355][ T1] ? check_init_srcu_struct (kernel/rcu/srcutree.c:408)
> [ 76.657792][ T1] synchronize_srcu (kernel/rcu/srcutree.c:1167 kernel/rcu/srcutree.c:1458)
> [ 76.659048][ T1] gpiod_request_commit (drivers/gpio/gpiolib.c:127 drivers/gpio/gpiolib.c:2273)
> [ 76.660430][ T1] gpiochip_request_own_desc (drivers/gpio/gpiolib.c:2454)
> [ 76.661898][ T1] gpiod_hog (drivers/gpio/gpiolib.c:4504)
> [ 76.663000][ T1] of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:799)
> [ 76.664334][ T1] ? __pfx_of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:785)
> [ 76.665830][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360)
> [ 76.667387][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423)
> [ 76.668763][ T1] ? _raw_spin_unlock_irqrestore (arch/x86/include/asm/preempt.h:103 include/linux/spinlock_api_smp.h:152 kernel/locking/spinlock.c:194)
> [ 76.670279][ T1] of_gpiochip_add (drivers/gpio/gpiolib-of.c:828 drivers/gpio/gpiolib-of.c:1143)
> [ 76.671550][ T1] ? fwnode_property_read_int_array (drivers/base/property.c:268 (discriminator 5))
> [ 76.673171][ T1] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:989)
> [ 76.674714][ T1] ? kasan_save_track (arch/x86/include/asm/current.h:42 mm/kasan/common.c:60 mm/kasan/common.c:70)
> [ 76.676002][ T1] unittest_gpio_probe (drivers/of/unittest.c:1886)
> [ 76.677340][ T1] platform_probe (drivers/base/platform.c:1404)
> [ 76.678544][ T1] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658)
> [ 76.679745][ T1] __driver_probe_device (drivers/base/dd.c:800)
> [ 76.681126][ T1] driver_probe_device (drivers/base/dd.c:830)
> [ 76.682411][ T1] __driver_attach (drivers/base/dd.c:1217)
> [ 76.683679][ T1] ? __pfx___driver_attach (drivers/base/dd.c:1157)
> [ 76.685074][ T1] bus_for_each_dev (drivers/base/bus.c:367)
> [ 76.686340][ T1] ? lockdep_init_map_type (kernel/locking/lockdep.c:4892)
> [ 76.687774][ T1] ? __pfx_bus_for_each_dev (drivers/base/bus.c:356)
> [ 76.689176][ T1] ? bus_add_driver (drivers/base/bus.c:672)
> [ 76.690452][ T1] bus_add_driver (drivers/base/bus.c:674)
> [ 76.691691][ T1] driver_register (drivers/base/driver.c:246)
> [ 76.692949][ T1] of_unittest_overlay_gpio (drivers/of/unittest.c:1969 (discriminator 4))
> [ 76.694377][ T1] of_unittest_overlay (drivers/of/unittest.c:2189 drivers/of/unittest.c:3217)
> [ 76.695724][ T1] ? __pfx_of_unittest_overlay (drivers/of/unittest.c:3155)
> [ 76.697198][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:4360)
> [ 76.698764][ T1] ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4423)
> [ 76.700151][ T1] of_unittest (drivers/of/unittest.c:4129)
> [ 76.701323][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080)
> [ 76.702622][ T1] ? add_device_randomness (drivers/char/random.c:918)
> [ 76.704050][ T1] ? __pfx_of_unittest (drivers/of/unittest.c:4080)
> [ 76.705347][ T1] do_one_initcall (init/main.c:1236)
> [ 76.706572][ T1] ? __pfx_do_one_initcall (init/main.c:1227)
> [ 76.708227][ T1] do_initcalls (init/main.c:1297 init/main.c:1314)
>
>
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20240212/202402122234.d85cca9b-lkp@intel.com
>
>
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Paul,

Could you help me out here? It seems that lockdep complains (with
"suspicious RCU usage") whenever an RCU-protected pointer is passed to
rcu_dereference() but is not actually dereferenced later - in which
case switching to rcu_access_pointer() helps. But in the case of the
of_unittests() it also emits the same warning for
gpiod_direction_input() where gdev->chip is fetched with
rcu_dereference() using CLASS(gpio_chip_guard) and later actually
dereferenced by calling guard.gc->...

Any hints as to what I'm doing wrong?

Thanks,
Bartosz
Bartosz Golaszewski Feb. 13, 2024, 8:10 a.m. UTC | #4
On Mon, Feb 12, 2024 at 10:20 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>

[snip]

> >
> >
> > [   76.432519][    T1] gpiochip_find_base_unlocked: found new base at 512
> > [   76.434591][    T1]
> > [   76.435240][    T1] =============================
> > [   76.436545][    T1] WARNING: suspicious RCU usage
> > [   76.437813][    T1] 6.8.0-rc1-00050-gc21131f83abc #1 Tainted: G                 N
> > [   76.439873][    T1] -----------------------------
> > [   76.441158][    T1] drivers/gpio/gpiolib.c:219 suspicious rcu_dereference_check() usage!
> > [   76.443364][    T1]

[snip]

>
> Paul,
>
> Could you help me out here? It seems that lockdep complains (with
> "suspicious RCU usage") whenever an RCU-protected pointer is passed to
> rcu_dereference() but is not actually dereferenced later - in which
> case switching to rcu_access_pointer() helps. But in the case of the
> of_unittests() it also emits the same warning for
> gpiod_direction_input() where gdev->chip is fetched with
> rcu_dereference() using CLASS(gpio_chip_guard) and later actually
> dereferenced by calling guard.gc->...
>
> Any hints as to what I'm doing wrong?
>
> Thanks,
> Bartosz

Seems like these can be silenced with rcu_dereference_protected() so
I'll use it for now.

Bart
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index e993c6a7215a..ccdeed013f6b 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -205,9 +205,9 @@  static long linehandle_ioctl(struct file *file, unsigned int cmd,
 	unsigned int i;
 	int ret;
 
-	guard(rwsem_read)(&lh->gdev->sem);
+	guard(srcu)(&lh->gdev->srcu);
 
-	if (!lh->gdev->chip)
+	if (!rcu_dereference(lh->gdev->chip))
 		return -ENODEV;
 
 	switch (cmd) {
@@ -1520,9 +1520,9 @@  static long linereq_ioctl(struct file *file, unsigned int cmd,
 	struct linereq *lr = file->private_data;
 	void __user *ip = (void __user *)arg;
 
-	guard(rwsem_read)(&lr->gdev->sem);
+	guard(srcu)(&lr->gdev->srcu);
 
-	if (!lr->gdev->chip)
+	if (!rcu_dereference(lr->gdev->chip))
 		return -ENODEV;
 
 	switch (cmd) {
@@ -1551,9 +1551,9 @@  static __poll_t linereq_poll(struct file *file,
 	struct linereq *lr = file->private_data;
 	__poll_t events = 0;
 
-	guard(rwsem_read)(&lr->gdev->sem);
+	guard(srcu)(&lr->gdev->srcu);
 
-	if (!lr->gdev->chip)
+	if (!rcu_dereference(lr->gdev->chip))
 		return EPOLLHUP | EPOLLERR;
 
 	poll_wait(file, &lr->wait, wait);
@@ -1573,9 +1573,9 @@  static ssize_t linereq_read(struct file *file, char __user *buf,
 	ssize_t bytes_read = 0;
 	int ret;
 
-	guard(rwsem_read)(&lr->gdev->sem);
+	guard(srcu)(&lr->gdev->srcu);
 
-	if (!lr->gdev->chip)
+	if (!rcu_dereference(lr->gdev->chip))
 		return -ENODEV;
 
 	if (count < sizeof(le))
@@ -1874,9 +1874,9 @@  static __poll_t lineevent_poll(struct file *file,
 	struct lineevent_state *le = file->private_data;
 	__poll_t events = 0;
 
-	guard(rwsem_read)(&le->gdev->sem);
+	guard(srcu)(&le->gdev->srcu);
 
-	if (!le->gdev->chip)
+	if (!rcu_dereference(le->gdev->chip))
 		return EPOLLHUP | EPOLLERR;
 
 	poll_wait(file, &le->wait, wait);
@@ -1912,9 +1912,9 @@  static ssize_t lineevent_read(struct file *file, char __user *buf,
 	ssize_t ge_size;
 	int ret;
 
-	guard(rwsem_read)(&le->gdev->sem);
+	guard(srcu)(&le->gdev->srcu);
 
-	if (!le->gdev->chip)
+	if (!rcu_dereference(le->gdev->chip))
 		return -ENODEV;
 
 	/*
@@ -1995,9 +1995,9 @@  static long lineevent_ioctl(struct file *file, unsigned int cmd,
 	void __user *ip = (void __user *)arg;
 	struct gpiohandle_data ghd;
 
-	guard(rwsem_read)(&le->gdev->sem);
+	guard(srcu)(&le->gdev->srcu);
 
-	if (!le->gdev->chip)
+	if (!rcu_dereference(le->gdev->chip))
 		return -ENODEV;
 
 	/*
@@ -2295,10 +2295,13 @@  static void gpio_v2_line_info_changed_to_v1(
 static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
 				  struct gpio_v2_line_info *info)
 {
-	struct gpio_chip *gc = desc->gdev->chip;
 	unsigned long dflags;
 	const char *label;
 
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return;
+
 	memset(info, 0, sizeof(*info));
 	info->offset = gpio_chip_hwgpio(desc);
 
@@ -2331,8 +2334,8 @@  static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
 	    test_bit(FLAG_USED_AS_IRQ, &dflags) ||
 	    test_bit(FLAG_EXPORT, &dflags) ||
 	    test_bit(FLAG_SYSFS, &dflags) ||
-	    !gpiochip_line_is_valid(gc, info->offset) ||
-	    !pinctrl_gpio_can_use_line(gc, info->offset))
+	    !gpiochip_line_is_valid(guard.gc, info->offset) ||
+	    !pinctrl_gpio_can_use_line(guard.gc, info->offset))
 		info->flags |= GPIO_V2_LINE_FLAG_USED;
 
 	if (test_bit(FLAG_IS_OUT, &dflags))
@@ -2505,10 +2508,10 @@  static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	struct gpio_device *gdev = cdev->gdev;
 	void __user *ip = (void __user *)arg;
 
-	guard(rwsem_read)(&gdev->sem);
+	guard(srcu)(&gdev->srcu);
 
 	/* We fail any subsequent ioctl():s when the chip is gone */
-	if (!gdev->chip)
+	if (!rcu_dereference(gdev->chip))
 		return -ENODEV;
 
 	/* Fill in the struct and pass to userspace */
@@ -2591,9 +2594,9 @@  static __poll_t lineinfo_watch_poll(struct file *file,
 	struct gpio_chardev_data *cdev = file->private_data;
 	__poll_t events = 0;
 
-	guard(rwsem_read)(&cdev->gdev->sem);
+	guard(srcu)(&cdev->gdev->srcu);
 
-	if (!cdev->gdev->chip)
+	if (!rcu_dereference(cdev->gdev->chip))
 		return EPOLLHUP | EPOLLERR;
 
 	poll_wait(file, &cdev->wait, pollt);
@@ -2614,9 +2617,9 @@  static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
 	int ret;
 	size_t event_size;
 
-	guard(rwsem_read)(&cdev->gdev->sem);
+	guard(srcu)(&cdev->gdev->srcu);
 
-	if (!cdev->gdev->chip)
+	if (!rcu_dereference(cdev->gdev->chip))
 		return -ENODEV;
 
 #ifndef CONFIG_GPIO_CDEV_V1
@@ -2691,10 +2694,10 @@  static int gpio_chrdev_open(struct inode *inode, struct file *file)
 	struct gpio_chardev_data *cdev;
 	int ret = -ENOMEM;
 
-	guard(rwsem_read)(&gdev->sem);
+	guard(srcu)(&gdev->srcu);
 
 	/* Fail on open if the backing gpiochip is gone */
-	if (!gdev->chip)
+	if (!rcu_dereference(gdev->chip))
 		return -ENODEV;
 
 	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
@@ -2781,6 +2784,7 @@  static const struct file_operations gpio_fileops = {
 
 int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
 {
+	struct gpio_chip *gc;
 	int ret;
 
 	cdev_init(&gdev->chrdev, &gpio_fileops);
@@ -2791,8 +2795,13 @@  int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
 	if (ret)
 		return ret;
 
-	chip_dbg(gdev->chip, "added GPIO chardev (%d:%d)\n",
-		 MAJOR(devt), gdev->id);
+	guard(srcu)(&gdev->srcu);
+
+	gc = rcu_dereference(gdev->chip);
+	if (!gc)
+		return -ENODEV;
+
+	chip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);
 
 	return 0;
 }
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 2e3edf41e853..9f7b5e20fc25 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -171,6 +171,10 @@  static int gpio_sysfs_request_irq(struct device *dev, unsigned char flags)
 	unsigned long irq_flags;
 	int ret;
 
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
 	data->irq = gpiod_to_irq(desc);
 	if (data->irq < 0)
 		return -EIO;
@@ -195,7 +199,7 @@  static int gpio_sysfs_request_irq(struct device *dev, unsigned char flags)
 	 *        Remove this redundant call (along with the corresponding
 	 *        unlock) when those drivers have been fixed.
 	 */
-	ret = gpiochip_lock_as_irq(desc->gdev->chip, gpio_chip_hwgpio(desc));
+	ret = gpiochip_lock_as_irq(guard.gc, gpio_chip_hwgpio(desc));
 	if (ret < 0)
 		goto err_put_kn;
 
@@ -209,7 +213,7 @@  static int gpio_sysfs_request_irq(struct device *dev, unsigned char flags)
 	return 0;
 
 err_unlock:
-	gpiochip_unlock_as_irq(desc->gdev->chip, gpio_chip_hwgpio(desc));
+	gpiochip_unlock_as_irq(guard.gc, gpio_chip_hwgpio(desc));
 err_put_kn:
 	sysfs_put(data->value_kn);
 
@@ -225,9 +229,13 @@  static void gpio_sysfs_free_irq(struct device *dev)
 	struct gpiod_data *data = dev_get_drvdata(dev);
 	struct gpio_desc *desc = data->desc;
 
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return;
+
 	data->irq_flags = 0;
 	free_irq(data->irq, data);
-	gpiochip_unlock_as_irq(desc->gdev->chip, gpio_chip_hwgpio(desc));
+	gpiochip_unlock_as_irq(guard.gc, gpio_chip_hwgpio(desc));
 	sysfs_put(data->value_kn);
 }
 
@@ -444,13 +452,12 @@  static ssize_t export_store(const struct class *class,
 				const char *buf, size_t len)
 {
 	struct gpio_desc *desc;
-	struct gpio_chip *gc;
 	int status, offset;
 	long gpio;
 
 	status = kstrtol(buf, 0, &gpio);
-	if (status < 0)
-		goto done;
+	if (status)
+		return status;
 
 	desc = gpio_to_desc(gpio);
 	/* reject invalid GPIOs */
@@ -458,9 +465,13 @@  static ssize_t export_store(const struct class *class,
 		pr_warn("%s: invalid GPIO %ld\n", __func__, gpio);
 		return -EINVAL;
 	}
-	gc = desc->gdev->chip;
+
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
 	offset = gpio_chip_hwgpio(desc);
-	if (!gpiochip_line_is_valid(gc, offset)) {
+	if (!gpiochip_line_is_valid(guard.gc, offset)) {
 		pr_warn("%s: GPIO %ld masked\n", __func__, gpio);
 		return -EINVAL;
 	}
@@ -563,7 +574,6 @@  int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 	const char *ioname = NULL;
 	struct gpio_device *gdev;
 	struct gpiod_data *data;
-	struct gpio_chip *chip;
 	struct device *dev;
 	int status, offset;
 
@@ -578,16 +588,19 @@  int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 		return -EINVAL;
 	}
 
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
 	if (!test_and_set_bit(FLAG_EXPORT, &desc->flags))
 		return -EPERM;
 
 	gdev = desc->gdev;
-	chip = gdev->chip;
 
 	mutex_lock(&sysfs_lock);
 
 	/* check if chip is being removed */
-	if (!chip || !gdev->mockdev) {
+	if (!gdev->mockdev) {
 		status = -ENODEV;
 		goto err_unlock;
 	}
@@ -606,14 +619,14 @@  int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 
 	data->desc = desc;
 	mutex_init(&data->mutex);
-	if (chip->direction_input && chip->direction_output)
+	if (guard.gc->direction_input && guard.gc->direction_output)
 		data->direction_can_change = direction_may_change;
 	else
 		data->direction_can_change = false;
 
 	offset = gpio_chip_hwgpio(desc);
-	if (chip->names && chip->names[offset])
-		ioname = chip->names[offset];
+	if (guard.gc->names && guard.gc->names[offset])
+		ioname = guard.gc->names[offset];
 
 	dev = device_create_with_groups(&gpio_class, &gdev->dev,
 					MKDEV(0, 0), data, gpio_groups,
@@ -728,7 +741,7 @@  EXPORT_SYMBOL_GPL(gpiod_unexport);
 
 int gpiochip_sysfs_register(struct gpio_device *gdev)
 {
-	struct gpio_chip *chip = gdev->chip;
+	struct gpio_chip *chip;
 	struct device *parent;
 	struct device *dev;
 
@@ -741,6 +754,12 @@  int gpiochip_sysfs_register(struct gpio_device *gdev)
 	if (!class_is_registered(&gpio_class))
 		return 0;
 
+	guard(srcu)(&gdev->srcu);
+
+	chip = rcu_dereference(gdev->chip);
+	if (!chip)
+		return -ENODEV;
+
 	/*
 	 * For sysfs backward compatibility we need to preserve this
 	 * preferred parenting to the gpio_chip parent field, if set.
@@ -767,7 +786,7 @@  int gpiochip_sysfs_register(struct gpio_device *gdev)
 void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 {
 	struct gpio_desc *desc;
-	struct gpio_chip *chip = gdev->chip;
+	struct gpio_chip *chip;
 
 	scoped_guard(mutex, &sysfs_lock) {
 		if (!gdev->mockdev)
@@ -779,6 +798,12 @@  void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 		gdev->mockdev = NULL;
 	}
 
+	guard(srcu)(&gdev->srcu);
+
+	chip = rcu_dereference(gdev->chip);
+	if (chip)
+		return;
+
 	/* unregister gpiod class devices owned by sysfs */
 	for_each_gpio_desc_with_flag(chip, desc, FLAG_SYSFS) {
 		gpiod_unexport(desc);
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 9b1907f3e400..a14717a3e222 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -216,7 +216,7 @@  struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
 {
 	if (!desc)
 		return NULL;
-	return desc->gdev->chip;
+	return rcu_dereference(desc->gdev->chip);
 }
 EXPORT_SYMBOL_GPL(gpiod_to_chip);
 
@@ -285,7 +285,7 @@  EXPORT_SYMBOL(gpio_device_get_label);
  */
 struct gpio_chip *gpio_device_get_chip(struct gpio_device *gdev)
 {
-	return gdev->chip;
+	return rcu_dereference(gdev->chip);
 }
 EXPORT_SYMBOL_GPL(gpio_device_get_chip);
 
@@ -325,12 +325,21 @@  static int gpiochip_find_base_unlocked(int ngpio)
  */
 int gpiod_get_direction(struct gpio_desc *desc)
 {
-	struct gpio_chip *gc;
 	unsigned long flags;
 	unsigned int offset;
 	int ret;
 
-	gc = gpiod_to_chip(desc);
+	/*
+	 * We cannot use VALIDATE_DESC() as we must not return 0 for a NULL
+	 * descriptor like we usually do.
+	 */
+	if (!desc || IS_ERR(desc))
+		return -EINVAL;
+
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
 	offset = gpio_chip_hwgpio(desc);
 	flags = READ_ONCE(desc->flags);
 
@@ -342,10 +351,10 @@  int gpiod_get_direction(struct gpio_desc *desc)
 	    test_bit(FLAG_IS_OUT, &flags))
 		return 0;
 
-	if (!gc->get_direction)
+	if (!guard.gc->get_direction)
 		return -ENOTSUPP;
 
-	ret = gc->get_direction(gc, offset);
+	ret = guard.gc->get_direction(guard.gc, offset);
 	if (ret < 0)
 		return ret;
 
@@ -421,6 +430,7 @@  static struct gpio_desc *gpio_name_to_desc(const char * const name)
 {
 	struct gpio_device *gdev;
 	struct gpio_desc *desc;
+	struct gpio_chip *gc;
 
 	if (!name)
 		return NULL;
@@ -429,7 +439,13 @@  static struct gpio_desc *gpio_name_to_desc(const char * const name)
 
 	list_for_each_entry_srcu(gdev, &gpio_devices, list,
 				 srcu_read_lock_held(&gpio_devices_srcu)) {
-		for_each_gpio_desc(gdev->chip, desc) {
+		guard(srcu)(&gdev->srcu);
+
+		gc = rcu_dereference(gdev->chip);
+		if (!gc)
+			continue;
+
+		for_each_gpio_desc(gc, desc) {
 			if (desc->name && !strcmp(desc->name, name))
 				return desc;
 		}
@@ -853,7 +869,7 @@  int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	gdev->dev.type = &gpio_dev_type;
 	gdev->dev.bus = &gpio_bus_type;
 	gdev->dev.parent = gc->parent;
-	gdev->chip = gc;
+	rcu_assign_pointer(gdev->chip, gc);
 
 	gc->gpiodev = gdev;
 	gpiochip_set_data(gc, data);
@@ -1097,7 +1113,8 @@  void gpiochip_remove(struct gpio_chip *gc)
 	synchronize_srcu(&gpio_devices_srcu);
 
 	/* Numb the device, cancelling all outstanding operations */
-	gdev->chip = NULL;
+	rcu_assign_pointer(gdev->chip, NULL);
+	synchronize_srcu(&gdev->srcu);
 	gpiochip_irqchip_remove(gc);
 	acpi_gpiochip_remove(gc);
 	of_gpiochip_remove(gc);
@@ -1156,6 +1173,7 @@  struct gpio_device *gpio_device_find(void *data,
 						  void *data))
 {
 	struct gpio_device *gdev;
+	struct gpio_chip *gc;
 
 	/*
 	 * Not yet but in the future the spinlock below will become a mutex.
@@ -1166,8 +1184,13 @@  struct gpio_device *gpio_device_find(void *data,
 
 	guard(srcu)(&gpio_devices_srcu);
 
-	list_for_each_entry(gdev, &gpio_devices, list) {
-		if (gdev->chip && match(gdev->chip, data))
+	list_for_each_entry_srcu(gdev, &gpio_devices, list,
+				 srcu_read_lock_held(&gpio_devices_srcu)) {
+		guard(srcu)(&gdev->srcu);
+
+		gc = rcu_dereference(gdev->chip);
+
+		if (gc && match(gc, data))
 			return gpio_device_get(gdev);
 	}
 
@@ -2214,10 +2237,13 @@  EXPORT_SYMBOL_GPL(gpiochip_remove_pin_ranges);
  */
 static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
 {
-	struct gpio_chip *gc = desc->gdev->chip;
 	unsigned int offset;
 	int ret;
 
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
 	if (test_and_set_bit(FLAG_REQUESTED, &desc->flags))
 		return -EBUSY;
 
@@ -2231,17 +2257,17 @@  static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
 	 * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
 	 */
 
-	if (gc->request) {
+	if (guard.gc->request) {
 		offset = gpio_chip_hwgpio(desc);
-		if (gpiochip_line_is_valid(gc, offset))
-			ret = gc->request(gc, offset);
+		if (gpiochip_line_is_valid(guard.gc, offset))
+			ret = guard.gc->request(guard.gc, offset);
 		else
 			ret = -EINVAL;
 		if (ret)
 			goto out_clear_bit;
 	}
 
-	if (gc->get_direction)
+	if (guard.gc->get_direction)
 		gpiod_get_direction(desc);
 
 	ret = desc_set_label(desc, label ? : "?");
@@ -2308,18 +2334,18 @@  int gpiod_request(struct gpio_desc *desc, const char *label)
 
 static bool gpiod_free_commit(struct gpio_desc *desc)
 {
-	struct gpio_chip *gc;
 	unsigned long flags;
 	bool ret = false;
 
 	might_sleep();
 
-	gc = desc->gdev->chip;
+	CLASS(gpio_chip_guard, guard)(desc);
+
 	flags = READ_ONCE(desc->flags);
 
-	if (gc && test_bit(FLAG_REQUESTED, &flags)) {
-		if (gc->free)
-			gc->free(gc, gpio_chip_hwgpio(desc));
+	if (guard.gc && test_bit(FLAG_REQUESTED, &flags)) {
+		if (guard.gc->free)
+			guard.gc->free(guard.gc, gpio_chip_hwgpio(desc));
 
 		clear_bit(FLAG_ACTIVE_LOW, &flags);
 		clear_bit(FLAG_REQUESTED, &flags);
@@ -2476,11 +2502,14 @@  static int gpio_set_config_with_argument(struct gpio_desc *desc,
 					 enum pin_config_param mode,
 					 u32 argument)
 {
-	struct gpio_chip *gc = desc->gdev->chip;
 	unsigned long config;
 
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
 	config = pinconf_to_config_packed(mode, argument);
-	return gpio_do_set_config(gc, gpio_chip_hwgpio(desc), config);
+	return gpio_do_set_config(guard.gc, gpio_chip_hwgpio(desc), config);
 }
 
 static int gpio_set_config_with_argument_optional(struct gpio_desc *desc,
@@ -2570,18 +2599,20 @@  int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce)
  */
 int gpiod_direction_input(struct gpio_desc *desc)
 {
-	struct gpio_chip *gc;
 	int ret = 0;
 
 	VALIDATE_DESC(desc);
-	gc = desc->gdev->chip;
+
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
 
 	/*
 	 * It is legal to have no .get() and .direction_input() specified if
 	 * the chip is output-only, but you can't specify .direction_input()
 	 * and not support the .get() operation, that doesn't make sense.
 	 */
-	if (!gc->get && gc->direction_input) {
+	if (!guard.gc->get && guard.gc->direction_input) {
 		gpiod_warn(desc,
 			   "%s: missing get() but have direction_input()\n",
 			   __func__);
@@ -2594,10 +2625,12 @@  int gpiod_direction_input(struct gpio_desc *desc)
 	 * direction (if .get_direction() is supported) else we silently
 	 * assume we are in input mode after this.
 	 */
-	if (gc->direction_input) {
-		ret = gc->direction_input(gc, gpio_chip_hwgpio(desc));
-	} else if (gc->get_direction &&
-		  (gc->get_direction(gc, gpio_chip_hwgpio(desc)) != 1)) {
+	if (guard.gc->direction_input) {
+		ret = guard.gc->direction_input(guard.gc,
+						gpio_chip_hwgpio(desc));
+	} else if (guard.gc->get_direction &&
+		  (guard.gc->get_direction(guard.gc,
+					   gpio_chip_hwgpio(desc)) != 1)) {
 		gpiod_warn(desc,
 			   "%s: missing direction_input() operation and line is output\n",
 			   __func__);
@@ -2616,28 +2649,31 @@  EXPORT_SYMBOL_GPL(gpiod_direction_input);
 
 static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
 {
-	struct gpio_chip *gc = desc->gdev->chip;
-	int val = !!value;
-	int ret = 0;
+	int val = !!value, ret = 0;
+
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
 
 	/*
 	 * It's OK not to specify .direction_output() if the gpiochip is
 	 * output-only, but if there is then not even a .set() operation it
 	 * is pretty tricky to drive the output line.
 	 */
-	if (!gc->set && !gc->direction_output) {
+	if (!guard.gc->set && !guard.gc->direction_output) {
 		gpiod_warn(desc,
 			   "%s: missing set() and direction_output() operations\n",
 			   __func__);
 		return -EIO;
 	}
 
-	if (gc->direction_output) {
-		ret = gc->direction_output(gc, gpio_chip_hwgpio(desc), val);
+	if (guard.gc->direction_output) {
+		ret = guard.gc->direction_output(guard.gc,
+						 gpio_chip_hwgpio(desc), val);
 	} else {
 		/* Check that we are in output mode if we can */
-		if (gc->get_direction &&
-		    gc->get_direction(gc, gpio_chip_hwgpio(desc))) {
+		if (guard.gc->get_direction &&
+		    guard.gc->get_direction(guard.gc, gpio_chip_hwgpio(desc))) {
 			gpiod_warn(desc,
 				"%s: missing direction_output() operation\n",
 				__func__);
@@ -2647,7 +2683,7 @@  static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
 		 * If we can't actively set the direction, we are some
 		 * output-only chip, so just drive the output as desired.
 		 */
-		gc->set(gc, gpio_chip_hwgpio(desc), val);
+		guard.gc->set(guard.gc, gpio_chip_hwgpio(desc), val);
 	}
 
 	if (!ret)
@@ -2763,17 +2799,20 @@  EXPORT_SYMBOL_GPL(gpiod_direction_output);
 int gpiod_enable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags)
 {
 	int ret = 0;
-	struct gpio_chip *gc;
 
 	VALIDATE_DESC(desc);
 
-	gc = desc->gdev->chip;
-	if (!gc->en_hw_timestamp) {
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
+	if (!guard.gc->en_hw_timestamp) {
 		gpiod_warn(desc, "%s: hw ts not supported\n", __func__);
 		return -ENOTSUPP;
 	}
 
-	ret = gc->en_hw_timestamp(gc, gpio_chip_hwgpio(desc), flags);
+	ret = guard.gc->en_hw_timestamp(guard.gc,
+					gpio_chip_hwgpio(desc), flags);
 	if (ret)
 		gpiod_warn(desc, "%s: hw ts request failed\n", __func__);
 
@@ -2792,17 +2831,20 @@  EXPORT_SYMBOL_GPL(gpiod_enable_hw_timestamp_ns);
 int gpiod_disable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags)
 {
 	int ret = 0;
-	struct gpio_chip *gc;
 
 	VALIDATE_DESC(desc);
 
-	gc = desc->gdev->chip;
-	if (!gc->dis_hw_timestamp) {
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
+	if (!guard.gc->dis_hw_timestamp) {
 		gpiod_warn(desc, "%s: hw ts not supported\n", __func__);
 		return -ENOTSUPP;
 	}
 
-	ret = gc->dis_hw_timestamp(gc, gpio_chip_hwgpio(desc), flags);
+	ret = guard.gc->dis_hw_timestamp(guard.gc, gpio_chip_hwgpio(desc),
+					 flags);
 	if (ret)
 		gpiod_warn(desc, "%s: hw ts release failed\n", __func__);
 
@@ -2821,12 +2863,13 @@  EXPORT_SYMBOL_GPL(gpiod_disable_hw_timestamp_ns);
  */
 int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
 {
-	struct gpio_chip *gc;
-
 	VALIDATE_DESC(desc);
-	gc = desc->gdev->chip;
 
-	return gpio_do_set_config(gc, gpio_chip_hwgpio(desc), config);
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
+	return gpio_do_set_config(guard.gc, gpio_chip_hwgpio(desc), config);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_config);
 
@@ -2924,10 +2967,19 @@  static int gpio_chip_get_value(struct gpio_chip *gc, const struct gpio_desc *des
 
 static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
 {
+	struct gpio_device *gdev;
 	struct gpio_chip *gc;
 	int value;
 
-	gc = desc->gdev->chip;
+	/* FIXME Unable to use gpio_chip_guard due to const desc. */
+	gdev = desc->gdev;
+
+	guard(srcu)(&gdev->srcu);
+
+	gc = rcu_dereference(gdev->chip);
+	if (!gc)
+		return -ENODEV;
+
 	value = gpio_chip_get_value(gc, desc);
 	value = value < 0 ? value : !!value;
 	trace_gpio_value(desc_to_gpio(desc), 1, value);
@@ -2953,6 +3005,14 @@  static int gpio_chip_get_multiple(struct gpio_chip *gc,
 	return -EIO;
 }
 
+/* The 'other' chip must be protected with its GPIO device's SRCU. */
+static bool gpio_device_chip_cmp(struct gpio_device *gdev, struct gpio_chip *gc)
+{
+	guard(srcu)(&gdev->srcu);
+
+	return gc == rcu_dereference(gdev->chip);
+}
+
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  unsigned int array_size,
 				  struct gpio_desc **desc_array,
@@ -2990,33 +3050,36 @@  int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 	}
 
 	while (i < array_size) {
-		struct gpio_chip *gc = desc_array[i]->gdev->chip;
 		DECLARE_BITMAP(fastpath_mask, FASTPATH_NGPIO);
 		DECLARE_BITMAP(fastpath_bits, FASTPATH_NGPIO);
 		unsigned long *mask, *bits;
 		int first, j;
 
-		if (likely(gc->ngpio <= FASTPATH_NGPIO)) {
+		CLASS(gpio_chip_guard, guard)(desc_array[i]);
+		if (!guard.gc)
+			return -ENODEV;
+
+		if (likely(guard.gc->ngpio <= FASTPATH_NGPIO)) {
 			mask = fastpath_mask;
 			bits = fastpath_bits;
 		} else {
 			gfp_t flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
 
-			mask = bitmap_alloc(gc->ngpio, flags);
+			mask = bitmap_alloc(guard.gc->ngpio, flags);
 			if (!mask)
 				return -ENOMEM;
 
-			bits = bitmap_alloc(gc->ngpio, flags);
+			bits = bitmap_alloc(guard.gc->ngpio, flags);
 			if (!bits) {
 				bitmap_free(mask);
 				return -ENOMEM;
 			}
 		}
 
-		bitmap_zero(mask, gc->ngpio);
+		bitmap_zero(mask, guard.gc->ngpio);
 
 		if (!can_sleep)
-			WARN_ON(gc->can_sleep);
+			WARN_ON(guard.gc->can_sleep);
 
 		/* collect all inputs belonging to the same chip */
 		first = i;
@@ -3031,9 +3094,9 @@  int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				i = find_next_zero_bit(array_info->get_mask,
 						       array_size, i);
 		} while ((i < array_size) &&
-			 (desc_array[i]->gdev->chip == gc));
+			 gpio_device_chip_cmp(desc_array[i]->gdev, guard.gc));
 
-		ret = gpio_chip_get_multiple(gc, mask, bits);
+		ret = gpio_chip_get_multiple(guard.gc, mask, bits);
 		if (ret) {
 			if (mask != fastpath_mask)
 				bitmap_free(mask);
@@ -3174,14 +3237,16 @@  EXPORT_SYMBOL_GPL(gpiod_get_array_value);
  */
 static void gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
 {
-	int ret = 0;
-	struct gpio_chip *gc = desc->gdev->chip;
-	int offset = gpio_chip_hwgpio(desc);
+	int ret = 0, offset = gpio_chip_hwgpio(desc);
+
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return;
 
 	if (value) {
-		ret = gc->direction_input(gc, offset);
+		ret = guard.gc->direction_input(guard.gc, offset);
 	} else {
-		ret = gc->direction_output(gc, offset, 0);
+		ret = guard.gc->direction_output(guard.gc, offset, 0);
 		if (!ret)
 			set_bit(FLAG_IS_OUT, &desc->flags);
 	}
@@ -3199,16 +3264,18 @@  static void gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
  */
 static void gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value)
 {
-	int ret = 0;
-	struct gpio_chip *gc = desc->gdev->chip;
-	int offset = gpio_chip_hwgpio(desc);
+	int ret = 0, offset = gpio_chip_hwgpio(desc);
+
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return;
 
 	if (value) {
-		ret = gc->direction_output(gc, offset, 1);
+		ret = guard.gc->direction_output(guard.gc, offset, 1);
 		if (!ret)
 			set_bit(FLAG_IS_OUT, &desc->flags);
 	} else {
-		ret = gc->direction_input(gc, offset);
+		ret = guard.gc->direction_input(guard.gc, offset);
 	}
 	trace_gpio_direction(desc_to_gpio(desc), !value, ret);
 	if (ret < 0)
@@ -3219,11 +3286,12 @@  static void gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value
 
 static void gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)
 {
-	struct gpio_chip *gc;
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return;
 
-	gc = desc->gdev->chip;
 	trace_gpio_value(desc_to_gpio(desc), 0, value);
-	gc->set(gc, gpio_chip_hwgpio(desc), value);
+	guard.gc->set(guard.gc, gpio_chip_hwgpio(desc), value);
 }
 
 /*
@@ -3284,33 +3352,36 @@  int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 	}
 
 	while (i < array_size) {
-		struct gpio_chip *gc = desc_array[i]->gdev->chip;
 		DECLARE_BITMAP(fastpath_mask, FASTPATH_NGPIO);
 		DECLARE_BITMAP(fastpath_bits, FASTPATH_NGPIO);
 		unsigned long *mask, *bits;
 		int count = 0;
 
-		if (likely(gc->ngpio <= FASTPATH_NGPIO)) {
+		CLASS(gpio_chip_guard, guard)(desc_array[i]);
+		if (!guard.gc)
+			return -ENODEV;
+
+		if (likely(guard.gc->ngpio <= FASTPATH_NGPIO)) {
 			mask = fastpath_mask;
 			bits = fastpath_bits;
 		} else {
 			gfp_t flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
 
-			mask = bitmap_alloc(gc->ngpio, flags);
+			mask = bitmap_alloc(guard.gc->ngpio, flags);
 			if (!mask)
 				return -ENOMEM;
 
-			bits = bitmap_alloc(gc->ngpio, flags);
+			bits = bitmap_alloc(guard.gc->ngpio, flags);
 			if (!bits) {
 				bitmap_free(mask);
 				return -ENOMEM;
 			}
 		}
 
-		bitmap_zero(mask, gc->ngpio);
+		bitmap_zero(mask, guard.gc->ngpio);
 
 		if (!can_sleep)
-			WARN_ON(gc->can_sleep);
+			WARN_ON(guard.gc->can_sleep);
 
 		do {
 			struct gpio_desc *desc = desc_array[i];
@@ -3346,10 +3417,10 @@  int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 				i = find_next_zero_bit(array_info->set_mask,
 						       array_size, i);
 		} while ((i < array_size) &&
-			 (desc_array[i]->gdev->chip == gc));
+			 gpio_device_chip_cmp(desc_array[i]->gdev, guard.gc));
 		/* push collected bits to outputs */
 		if (count != 0)
-			gpio_chip_set_multiple(gc, mask, bits);
+			gpio_chip_set_multiple(guard.gc, mask, bits);
 
 		if (mask != fastpath_mask)
 			bitmap_free(mask);
@@ -3505,6 +3576,7 @@  EXPORT_SYMBOL_GPL(gpiod_set_consumer_name);
  */
 int gpiod_to_irq(const struct gpio_desc *desc)
 {
+	struct gpio_device *gdev;
 	struct gpio_chip *gc;
 	int offset;
 
@@ -3516,7 +3588,13 @@  int gpiod_to_irq(const struct gpio_desc *desc)
 	if (!desc || IS_ERR(desc))
 		return -EINVAL;
 
-	gc = desc->gdev->chip;
+	gdev = desc->gdev;
+	/* FIXME Cannot use gpio_chip_guard due to const desc. */
+	guard(srcu)(&gdev->srcu);
+	gc = rcu_dereference(gdev->chip);
+	if (!gc)
+		return -ENODEV;
+
 	offset = gpio_chip_hwgpio(desc);
 	if (gc->to_irq) {
 		int retirq = gc->to_irq(gc, offset);
@@ -4696,12 +4774,20 @@  core_initcall(gpiolib_dev_init);
 
 static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
 {
-	struct gpio_chip *gc = gdev->chip;
 	bool active_low, is_irq, is_out;
 	unsigned int gpio = gdev->base;
 	struct gpio_desc *desc;
+	struct gpio_chip *gc;
 	int value;
 
+	guard(srcu)(&gdev->srcu);
+
+	gc = rcu_dereference(gdev->chip);
+	if (!gc) {
+		seq_puts(s, "Underlying GPIO chip is gone\n");
+		return;
+	}
+
 	for_each_gpio_desc(gc, desc) {
 		guard(srcu)(&desc->srcu);
 		if (test_bit(FLAG_REQUESTED, &desc->flags)) {
@@ -4776,9 +4862,12 @@  static int gpiolib_seq_show(struct seq_file *s, void *v)
 {
 	struct gpiolib_seq_priv *priv = s->private;
 	struct gpio_device *gdev = v;
-	struct gpio_chip *gc = gdev->chip;
+	struct gpio_chip *gc;
 	struct device *parent;
 
+	guard(srcu)(&gdev->srcu);
+
+	gc = rcu_dereference(gdev->chip);
 	if (!gc) {
 		seq_printf(s, "%s%s: (dangling chip)",
 			   priv->newline ? "\n" : "",
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 35d71e30c546..b3810f7d286a 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -63,7 +63,7 @@  struct gpio_device {
 	int			id;
 	struct device		*mockdev;
 	struct module		*owner;
-	struct gpio_chip	*chip;
+	struct gpio_chip __rcu	*chip;
 	struct gpio_desc	*descs;
 	int			base;
 	u16			ngpio;
@@ -193,6 +193,26 @@  struct gpio_desc {
 
 #define gpiod_not_found(desc)		(IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
 
+struct gpio_chip_guard {
+	struct gpio_device *gdev;
+	struct gpio_chip *gc;
+	int idx;
+};
+
+DEFINE_CLASS(gpio_chip_guard,
+	     struct gpio_chip_guard,
+	     srcu_read_unlock(&_T.gdev->srcu, _T.idx),
+	     ({
+		struct gpio_chip_guard _guard;
+
+		_guard.gdev = desc->gdev;
+		_guard.idx = srcu_read_lock(&_guard.gdev->srcu);
+		_guard.gc = rcu_dereference(_guard.gdev->chip);
+
+		_guard;
+	     }),
+	     struct gpio_desc *desc)
+
 int gpiod_request(struct gpio_desc *desc, const char *label);
 void gpiod_free(struct gpio_desc *desc);