On Wed, Jun 22 2022, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> We try to harden virtio device notifications in 8b4ec69d7e09 ("virtio:
> harden vring IRQ"). It works with the assumption that the driver or
> core can properly call virtio_device_ready() at the right
> place. Unfortunately, this seems to be not true and uncover various
> bugs of the existing drivers, mainly the issue of using
> virtio_device_ready() incorrectly.
>
> So let's having a Kconfig option and disable it by default. It gives
s/having/have/
> us a breath to fix the drivers and then we can consider to enable it
> by default.
>
> Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
> ---
> Changes since V2:
> - Tweak the Kconfig help
> - Add comment for the read_lock() pairing in virtio_ccw
> ---
> drivers/s390/virtio/virtio_ccw.c | 9 ++++++++-
> drivers/virtio/Kconfig | 13 +++++++++++++
> drivers/virtio/virtio.c | 2 ++
> drivers/virtio/virtio_ring.c | 12 ++++++++++++
> include/linux/virtio_config.h | 2 ++
> 5 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> index 97e51c34e6cf..1f6a358f65f0 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -1136,8 +1136,13 @@ static void virtio_ccw_int_handler(struct ccw_device *cdev,
> vcdev->err = -EIO;
> }
> virtio_ccw_check_activity(vcdev, activity);
> - /* Interrupts are disabled here */
> +#ifdef CONFIG_VIRTIO_HARDEN_NOTIFICATION
> + /*
> + * Paried with virtio_ccw_synchronize_cbs() and interrupts are
s/Paried/Paired/
> + * disabled here.
> + */
> read_lock(&vcdev->irq_lock);
> +#endif
> for_each_set_bit(i, indicators(vcdev),
> sizeof(*indicators(vcdev)) * BITS_PER_BYTE) {
> /* The bit clear must happen before the vring kick. */
> @@ -1146,7 +1151,9 @@ static void virtio_ccw_int_handler(struct ccw_device *cdev,
> vq = virtio_ccw_vq_by_ind(vcdev, i);
> vring_interrupt(0, vq);
> }
> +#ifdef CONFIG_VIRTIO_HARDEN_NOTIFICATION
> read_unlock(&vcdev->irq_lock);
> +#endif
> if (test_bit(0, indicators2(vcdev))) {
> virtio_config_changed(&vcdev->vdev);
> clear_bit(0, indicators2(vcdev));
Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
