linux-kernel - Re: [PATCH] virtio: Remove virtio devices on device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cc7312ca-2745-48a5-a5ac-9ee1d1c4bee0@redhat.com>
Date: Fri, 31 Jan 2025 10:53:15 +0100
From: Eric Auger <eauger@...hat.com>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
 "Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez
 <eperezma@...hat.com>, Zhenzhong Duan <zhenzhong.duan@...el.com>
Cc: virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org,
 Hongyu Ning <hongyu.ning@...ux.intel.com>
Subject: Re: [PATCH] virtio: Remove virtio devices on device_shutdown()

Hi Kirill, Michael

On 8/8/24 9:51 AM, Kirill A. Shutemov wrote:
> Hongyu reported a hang on kexec in a VM. QEMU reported invalid memory
> accesses during the hang.
> 
> 	Invalid read at addr 0x102877002, size 2, region '(null)', reason: rejected
> 	Invalid write at addr 0x102877A44, size 2, region '(null)', reason: rejected
> 	...
> 
> It was traced down to virtio-console. Kexec works fine if virtio-console
> is not in use.
> 
> Looks like virtio-console continues to write to the MMIO even after
> underlying virtio-pci device is removed.
> 
> The problem can be mitigated by removing all virtio devices on virtio
> bus shutdown.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Reported-by: Hongyu Ning <hongyu.ning@...ux.intel.com>

Gentle ping on that patch that seems to have fallen though the cracks.

I think this fix is really needed. I have another test case with a
rebooting guest exposed with virtio-net (backed by vhost-net) and
viommu. Since there is currently no shutdown for the virtio-net, on
reboot, the IOMMU is disabled through the native_machine_shutdown()/
x86_platform.iommu_shutdown() while the virtio-net is still alive.

Normally device_shutdown() should call virtio-net shutdown before the
IOMMU tear down and we wouldn't see any spurious transactions after
iommu shutdown.

With that fix, the above test case is fixed and I do not see spurious
vhost IOTLB miss spurious requests.

For more details, see qemu thread ([PATCH] hw/virtio/vhost: Disable
IOTLB callbacks when IOMMU gets disabled,
https://lore.kernel.org/all/20250120173339.865681-1-eric.auger@redhat.com/)

Reviewed-by: Eric Auger <eric.auger@...hat.com>
Tested-by: Eric Auger <eric.auger@...hat.com>

Thanks

Eric

> ---
>  drivers/virtio/virtio.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index a9b93e99c23a..6c2f908eb22c 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -356,6 +356,15 @@ static void virtio_dev_remove(struct device *_d)
>  	of_node_put(dev->dev.of_node);
>  }
>  
> +static void virtio_dev_shutdown(struct device *_d)
> +{
> +	struct virtio_device *dev = dev_to_virtio(_d);
> +	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
> +
> +	if (drv && drv->remove)
> +		drv->remove(dev);
> +}
> +
>  static const struct bus_type virtio_bus = {
>  	.name  = "virtio",
>  	.match = virtio_dev_match,
> @@ -363,6 +372,7 @@ static const struct bus_type virtio_bus = {
>  	.uevent = virtio_uevent,
>  	.probe = virtio_dev_probe,
>  	.remove = virtio_dev_remove,
> +	.shutdown = virtio_dev_shutdown,
>  };
>  
>  int __register_virtio_driver(struct virtio_driver *driver, struct module *owner)