While backporting an upstream patch to one of our older distro kernels I've
found a weird locking hierarchy in the virtio crypto driver:
virtio_config_changed():
spin_lock_irqsave()
->__virtio_config_changed()
->drv->config_changed() <- assume drv ==
virtio_crypto_driver
virtcrypto_config_changed():
->virtcrypto_update_status()
->virtcrypto_dev_start()
->virtio_crypto_skcipher_algs_register()
mutex_lock()
I don't think trying to take a mutex while holding a spinlock with
interrupts
disabled is a proper thing to do.
Am I overlooking something here?
Juergen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xB0DE9DD628BF132F.asc
Type: application/pgp-keys
Size: 3098 bytes
Desc: OpenPGP public key
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20230922/5d4d6c4e/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20230922/5d4d6c4e/attachment-0001.sig>
On Fri, Sep 22, 2023 at 10:36:59AM +0200, Juergen Gross wrote:> While backporting an upstream patch to one of our older distro kernels I've > found a weird locking hierarchy in the virtio crypto driver: > > virtio_config_changed(): > spin_lock_irqsave() > ->__virtio_config_changed() > ->drv->config_changed() <- assume drv == virtio_crypto_driver > virtcrypto_config_changed(): > ->virtcrypto_update_status() > ->virtcrypto_dev_start() > ->virtio_crypto_skcipher_algs_register() > mutex_lock() > > I don't think trying to take a mutex while holding a spinlock with interrupts > disabled is a proper thing to do. > > Am I overlooking something here? > > > JuergenYes - this seems like a bug in virtcrypto - for example, virt blk queues a work for this reason. It's so common that it might make sense to move the config work into virtio core. In any case, patches welcome. -- MST