Weird locking hierarchy in virtcrypto

Weird locking hierarchy in virtcrypto

[Juergen Gross via Virtualization]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 682 bytes --]

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

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: Weird locking hierarchy in virtcrypto

[Michael S. Tsirkin]

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?
> 
> 
> Juergen

Yes - 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

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization