Linux kernel documentation tree maintained by Jonathan Corbet http://mirrors.hust.edu.cn/git/lwn.git/atom?h=docs-6.4-2 2023-02-06T14:51:33+00:00 2023-02-06T14:51:33+00:00 Dan Carpenter error27@gmail.com 2023-02-06T14:47:38+00:00 urn:sha1:941811980df00161fe639c252dabe6488875887c This unlocks twice due to a merge issue. Fixes: 35bba23e6732 ("Merge remote-tracking branch 'spi/for-6.2' into spi-6.2") Signed-off-by: Dan Carpenter <error27@gmail.com> Link: https://lore.kernel.org/r/Y+ETCk7XKfAy8yWQ@kili Signed-off-by: Mark Brown <broonie@kernel.org> 2023-02-06T14:50:53+00:00 Mark Brown broonie@kernel.org 2023-02-06T14:50:53+00:00 urn:sha1:f8f5a7c902776c18598963b359046ddf0728be84 The automated merge for the spidev locking fixes is going wrong and merging the unlock twice. 2023-01-30T10:47:16+00:00 Mark Brown broonie@kernel.org 2023-01-30T10:47:16+00:00 urn:sha1:35bba23e6732a593722c96cbe03ced627f0d8831 2023-01-27T16:38:05+00:00 Bartosz Golaszewski bartosz.golaszewski@linaro.org 2023-01-16T14:41:49+00:00 urn:sha1:eede42c9459b58b71edc99303dad65216a655810 When calling spidev_message() from the one of the ioctl() callbacks, the spi_lock is already taken. When we then end up calling spidev_sync(), we get the following splat: [ 214.047619] [ 214.049198] ============================================ [ 214.054533] WARNING: possible recursive locking detected [ 214.059858] 6.2.0-rc3-0.0.0-devel+git.97ec4d559d93 #1 Not tainted [ 214.065969] -------------------------------------------- [ 214.071290] spidev_test/1454 is trying to acquire lock: [ 214.076530] c4925dbc (&spidev->spi_lock){+.+.}-{3:3}, at: spidev_ioctl+0x8e0/0xab8 [ 214.084164] [ 214.084164] but task is already holding lock: [ 214.090007] c4925dbc (&spidev->spi_lock){+.+.}-{3:3}, at: spidev_ioctl+0x44/0xab8 [ 214.097537] [ 214.097537] other info that might help us debug this: [ 214.104075] Possible unsafe locking scenario: [ 214.104075] [ 214.110004] CPU0 [ 214.112461] ---- [ 214.114916] lock(&spidev->spi_lock); [ 214.118687] lock(&spidev->spi_lock); [ 214.122457] [ 214.122457] *** DEADLOCK *** [ 214.122457] [ 214.128386] May be due to missing lock nesting notation [ 214.128386] [ 214.135183] 2 locks held by spidev_test/1454: [ 214.139553] #0: c4925dbc (&spidev->spi_lock){+.+.}-{3:3}, at: spidev_ioctl+0x44/0xab8 [ 214.147524] #1: c4925e14 (&spidev->buf_lock){+.+.}-{3:3}, at: spidev_ioctl+0x70/0xab8 [ 214.155493] [ 214.155493] stack backtrace: [ 214.159861] CPU: 0 PID: 1454 Comm: spidev_test Not tainted 6.2.0-rc3-0.0.0-devel+git.97ec4d559d93 #1 [ 214.169012] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) [ 214.175555] unwind_backtrace from show_stack+0x10/0x14 [ 214.180819] show_stack from dump_stack_lvl+0x60/0x90 [ 214.185900] dump_stack_lvl from __lock_acquire+0x874/0x2858 [ 214.191584] __lock_acquire from lock_acquire+0xfc/0x378 [ 214.196918] lock_acquire from __mutex_lock+0x9c/0x8a8 [ 214.202083] __mutex_lock from mutex_lock_nested+0x1c/0x24 [ 214.207597] mutex_lock_nested from spidev_ioctl+0x8e0/0xab8 [ 214.213284] spidev_ioctl from sys_ioctl+0x4d0/0xe2c [ 214.218277] sys_ioctl from ret_fast_syscall+0x0/0x1c [ 214.223351] Exception stack(0xe75cdfa8 to 0xe75cdff0) [ 214.228422] dfa0: 00000000 00001000 00000003 40206b00 bee266e8 bee266e0 [ 214.236617] dfc0: 00000000 00001000 006a71a0 00000036 004c0040 004bfd18 00000000 00000003 [ 214.244809] dfe0: 00000036 bee266c8 b6f16dc5 b6e8e5f6 Fix it by introducing an unlocked variant of spidev_sync() and calling it from spidev_message() while other users who don't check the spidev->spi's existence keep on using the locking flavor. Reported-by: Francesco Dolcini <francesco@dolcini.it> Fixes: 1f4d2dd45b6e ("spi: spidev: fix a race condition when accessing spidev->spi") Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Tested-by: Max Krummenacher <max.krummenacher@toradex.com> Link: https://lore.kernel.org/r/20230116144149.305560-1-brgl@bgdev.pl Signed-off-by: Mark Brown <broonie@kernel.org> 2023-01-20T12:06:28+00:00 Krzysztof Kozlowski krzysztof.kozlowski@linaro.org 2023-01-20T07:56:51+00:00 urn:sha1:be5852457b7e85ad13b1bded9c97bed5ee1715a3 Bring some order to reduce possibilities of conflicts. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Link: https://lore.kernel.org/r/20230120075651.153763-1-krzysztof.kozlowski@linaro.org Signed-off-by: Mark Brown <broonie@kernel.org> 2023-01-11T14:15:10+00:00 Bartosz Golaszewski bartosz.golaszewski@linaro.org 2023-01-06T10:07:19+00:00 urn:sha1:6b35b173dbc1711f8d272e3f322d2ad697015919 The two debug messages in spidev_open() dereference spidev->spi without taking the lock and without checking if it's not null. This can lead to a crash. Drop the messages as they're not needed - the user-space will get informed about ENOMEM with the syscall return value. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Link: https://lore.kernel.org/r/20230106100719.196243-2-brgl@bgdev.pl Signed-off-by: Mark Brown <broonie@kernel.org> 2023-01-11T14:15:10+00:00 Bartosz Golaszewski bartosz.golaszewski@linaro.org 2023-01-06T10:07:18+00:00 urn:sha1:a720416d94634068951773cb9e9d6f1b73769e5b There's a spinlock in place that is taken in file_operations callbacks whenever we check if spidev->spi is still alive (not null). It's also taken when spidev->spi is set to NULL in remove(). This however doesn't protect the code against driver unbind event while one of the syscalls is still in progress. To that end we need a lock taken continuously as long as we may still access spidev->spi. As both the file ops and the remove callback are never called from interrupt context, we can replace the spinlock with a mutex. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Link: https://lore.kernel.org/r/20230106100719.196243-1-brgl@bgdev.pl Signed-off-by: Mark Brown <broonie@kernel.org> 2023-01-06T17:45:43+00:00 Bartosz Golaszewski bartosz.golaszewski@linaro.org 2023-01-06T10:07:19+00:00 urn:sha1:50028988403ab8e031093655af9c6919ecba3aa6 The two debug messages in spidev_open() dereference spidev->spi without taking the lock and without checking if it's not null. This can lead to a crash. Drop the messages as they're not needed - the user-space will get informed about ENOMEM with the syscall return value. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Link: https://lore.kernel.org/r/20230106100719.196243-2-brgl@bgdev.pl Signed-off-by: Mark Brown <broonie@kernel.org> 2023-01-06T17:45:42+00:00 Bartosz Golaszewski bartosz.golaszewski@linaro.org 2023-01-06T10:07:18+00:00 urn:sha1:1f4d2dd45b6ef9c047f620d2812326a7813d2354 There's a spinlock in place that is taken in file_operations callbacks whenever we check if spidev->spi is still alive (not null). It's also taken when spidev->spi is set to NULL in remove(). This however doesn't protect the code against driver unbind event while one of the syscalls is still in progress. To that end we need a lock taken continuously as long as we may still access spidev->spi. As both the file ops and the remove callback are never called from interrupt context, we can replace the spinlock with a mutex. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Link: https://lore.kernel.org/r/20230106100719.196243-1-brgl@bgdev.pl Signed-off-by: Mark Brown <broonie@kernel.org> 2022-12-27T14:14:02+00:00 Vincent Tremblay vincent@vtremblay.dev 2022-12-27T14:10:08+00:00 urn:sha1:6c9d1fd52956c3148e847a214bae9102b1811de5 Add compatible string for Silicon Labs SI3210 device. Signed-off-by: Vincent Tremblay <vincent@vtremblay.dev> Link: https://lore.kernel.org/r/20221227141011.111410-2-vincent@vtremblay.dev Signed-off-by: Mark Brown <broonie@kernel.org>