Weird locking hierarchy in virtcrypto

Michael S. Tsirkin
2023-09-22T10:45:08Z

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