search for: virtio_pci_remove

...31a3ddda166cda86d2b5111e09ba4bda5239fae6 introduced a use after free in virtio-pci. The main issue is that the release method signals removal of the virtio device, while remove signals removal of the pci device. For example, on driver removal or hot-unplug, virtio_pci_release_dev is called before virtio_pci_remove. We then might get a crash as virtio_pci_remove tries to use the device freed by virtio_pci_release_dev. We allocate/free all resources together with the pci device, so we can leave the release method empty. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/virtio/virtio_pc...

...31a3ddda166cda86d2b5111e09ba4bda5239fae6 introduced a use after free in virtio-pci. The main issue is that the release method signals removal of the virtio device, while remove signals removal of the pci device. For example, on driver removal or hot-unplug, virtio_pci_release_dev is called before virtio_pci_remove. We then might get a crash as virtio_pci_remove tries to use the device freed by virtio_pci_release_dev. We allocate/free all resources together with the pci device, so we can leave the release method empty. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/virtio/virtio_pc...

KASan detected a use-after-free error in virtio-pci remove code. In virtio_pci_remove(), vp_dev is still used after being freed in unregister_virtio_device() (in virtio_pci_release_dev() more precisely). To fix, keep a reference until cleanup is done. Fixes: 63bd62a08ca4 ("virtio_pci: defer kfree until release callback") Reported-by: Jerome Marchand <jmarchan at redha...

KASan detected a use-after-free error in virtio-pci remove code. In virtio_pci_remove(), vp_dev is still used after being freed in unregister_virtio_device() (in virtio_pci_release_dev() more precisely). To fix, keep a reference until cleanup is done. Fixes: 63bd62a08ca4 ("virtio_pci: defer kfree until release callback") Reported-by: Jerome Marchand <jmarchan at redha...

...100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -743,6 +743,14 @@ out: return err; } +static void virtio_pci_shutdown(struct pci_dev *pci_dev) +{ + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); + + vp_reset(&vp_dev->vdev); +} + + static void virtio_pci_remove(struct pci_dev *pci_dev) { struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); @@ -794,6 +802,7 @@ static struct pci_driver virtio_pci_driver = { .id_table = virtio_pci_id_table, .probe = virtio_pci_probe, .remove = virtio_pci_remove, + .shutdown = virtio_pci_shutdown, #ifdef C...

...100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -743,6 +743,14 @@ out: return err; } +static void virtio_pci_shutdown(struct pci_dev *pci_dev) +{ + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); + + vp_reset(&vp_dev->vdev); +} + + static void virtio_pci_remove(struct pci_dev *pci_dev) { struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); @@ -794,6 +802,7 @@ static struct pci_driver virtio_pci_driver = { .id_table = virtio_pci_id_table, .probe = virtio_pci_probe, .remove = virtio_pci_remove, + .shutdown = virtio_pci_shutdown, #ifdef C...

..._pci_freeze, virtio_pci_restore) -}; #endif +static SIMPLE_DEV_PM_OPS(virtio_pci_pm_ops, virtio_pci_freeze, + virtio_pci_restore); + static struct pci_driver virtio_pci_driver = { .name = "virtio-pci", .id_table = virtio_pci_id_table, .probe = virtio_pci_probe, .remove = virtio_pci_remove, -#ifdef CONFIG_PM_SLEEP .driver.pm = &virtio_pci_pm_ops, -#endif }; module_pci_driver(virtio_pci_driver); -- 2.0.0

..._pci_freeze, virtio_pci_restore) -}; #endif +static SIMPLE_DEV_PM_OPS(virtio_pci_pm_ops, virtio_pci_freeze, + virtio_pci_restore); + static struct pci_driver virtio_pci_driver = { .name = "virtio-pci", .id_table = virtio_pci_id_table, .probe = virtio_pci_probe, .remove = virtio_pci_remove, -#ifdef CONFIG_PM_SLEEP .driver.pm = &virtio_pci_pm_ops, -#endif }; module_pci_driver(virtio_pci_driver); -- 2.0.0

...7 ++++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index 48d4d1cf1cb6..1d4467b2dc31 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -577,6 +577,8 @@ static void virtio_pci_remove(struct pci_dev *pci_dev) struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); struct device *dev = get_device(&vp_dev->vdev.dev); + pci_disable_sriov(pci_dev); + unregister_virtio_device(&vp_dev->vdev); if (vp_dev->ioaddr) @@ -588,6 +590,33 @@ static void virtio...

...7 ++++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index 48d4d1cf1cb6..1d4467b2dc31 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -577,6 +577,8 @@ static void virtio_pci_remove(struct pci_dev *pci_dev) struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); struct device *dev = get_device(&vp_dev->vdev.dev); + pci_disable_sriov(pci_dev); + unregister_virtio_device(&vp_dev->vdev); if (vp_dev->ioaddr) @@ -588,6 +590,33 @@ static void virtio...

...elease_dev() kicks in, as the last ref to the virtio device is now gone, and attempts to do pci_iounmap(pci_dev, vp_dev->ioaddr); pci_release_regions(pci_dev); pci_disable_device(pci_dev); which results in a double-free warning. Move the code that releases regions, etc., to the virtio_pci_remove() function, and all that's now left in release_dev is the final freeing of the vp_dev. Signed-off-by: Amit Shah <amit.shah at redhat.com> --- drivers/virtio/virtio_pci.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/virtio/virtio_pci.c b/...

...elease_dev() kicks in, as the last ref to the virtio device is now gone, and attempts to do pci_iounmap(pci_dev, vp_dev->ioaddr); pci_release_regions(pci_dev); pci_disable_device(pci_dev); which results in a double-free warning. Move the code that releases regions, etc., to the virtio_pci_remove() function, and all that's now left in release_dev is the final freeing of the vp_dev. Signed-off-by: Amit Shah <amit.shah at redhat.com> --- drivers/virtio/virtio_pci.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/virtio/virtio_pci.c b/...

...7 ++++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index 48d4d1cf1cb6..1d4467b2dc31 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -577,6 +577,8 @@ static void virtio_pci_remove(struct pci_dev *pci_dev) struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); struct device *dev = get_device(&vp_dev->vdev.dev); + pci_disable_sriov(pci_dev); + unregister_virtio_device(&vp_dev->vdev); if (vp_dev->ioaddr) @@ -588,6 +590,33 @@ static void virtio...

...7 ++++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index 48d4d1cf1cb6..1d4467b2dc31 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -577,6 +577,8 @@ static void virtio_pci_remove(struct pci_dev *pci_dev) struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); struct device *dev = get_device(&vp_dev->vdev.dev); + pci_disable_sriov(pci_dev); + unregister_virtio_device(&vp_dev->vdev); if (vp_dev->ioaddr) @@ -588,6 +590,33 @@ static void virtio...

...> > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > > index 48d4d1cf1cb6..1d4467b2dc31 100644 > > --- a/drivers/virtio/virtio_pci_common.c > > +++ b/drivers/virtio/virtio_pci_common.c > > @@ -577,6 +577,8 @@ static void virtio_pci_remove(struct pci_dev *pci_dev) > > struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); > > struct device *dev = get_device(&vp_dev->vdev.dev); > > > > + pci_disable_sriov(pci_dev); > > + > > unregister_virtio_device(&vp_dev->vdev); > &...

Fixes a couple of minor compliance issues in new virtio 1.0 code. Plus, adds a couple of minor cleanups - not bugfixes, but seem safe enough for 3.19. Michael S. Tsirkin (6): virtio: set VIRTIO_CONFIG_S_FEATURES_OK on restore virtio_config: fix virtio_cread_bytes virtio_pci_common.h: drop VIRTIO_PCI_NO_LEGACY virtio_pci: move probe to common file virtio_pci: add VIRTIO_PCI_NO_LEGACY

Fixes a couple of minor compliance issues in new virtio 1.0 code. Plus, adds a couple of minor cleanups - not bugfixes, but seem safe enough for 3.19. Michael S. Tsirkin (6): virtio: set VIRTIO_CONFIG_S_FEATURES_OK on restore virtio_config: fix virtio_cread_bytes virtio_pci_common.h: drop VIRTIO_PCI_NO_LEGACY virtio_pci: move probe to common file virtio_pci: add VIRTIO_PCI_NO_LEGACY

...s/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > > > > index 48d4d1cf1cb6..1d4467b2dc31 100644 > > > > --- a/drivers/virtio/virtio_pci_common.c > > > > +++ b/drivers/virtio/virtio_pci_common.c > > > > @@ -577,6 +577,8 @@ static void virtio_pci_remove(struct pci_dev *pci_dev) > > > > struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); > > > > struct device *dev = get_device(&vp_dev->vdev.dev); > > > > > > > > + pci_disable_sriov(pci_dev); > > > > + > > > &...

Hi Michael, KASan detected a use-after-free error in virtio-pci remove code. In virtio_pci_remove(), vp_dev is still used after being freed in unregister_virtio_device() (in virtio_pci_release_dev() more precisely). I don't know the proper way to fix this. Here is the KASan output: [ 467.987227] ================================================================== [ 467.990023] BUG: KASAN:...

Hi Michael, KASan detected a use-after-free error in virtio-pci remove code. In virtio_pci_remove(), vp_dev is still used after being freed in unregister_virtio_device() (in virtio_pci_release_dev() more precisely). I don't know the proper way to fix this. Here is the KASan output: [ 467.987227] ================================================================== [ 467.990023] BUG: KASAN:...