* [RFC PATCH 00/15] Handle virtio_device_ready() failure
@ 2021-05-17 9:34 Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 01/15] virtio_config: Add return value to virtio_device_ready() Xie Yongji
` (14 more replies)
0 siblings, 15 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now we would trigger a BUG_ON() when we get an invalid status in
virtio_device_ready() during probe and suspend/resume. But
returning invalid status might happen in VDUSE[1] case where
the device becomes untrusted. So this series tries to remove the
BUG_ON() and return error to the caller so that the device driver
can handle the error correctly.
Now this series is based on my another fix[2].
[1] https://lore.kernel.org/kvm/20210331080519.172-1-xieyongji@bytedance.com/
[2] https://lore.kernel.org/lkml/20210517083557.172-1-xieyongji@bytedance.com/
Xie Yongji (15):
virtio_config: Add return value to virtio_device_ready()
virtio-blk: Handle virtio_device_ready() failure
virtio_console: Handle virtio_device_ready() failure
virtio_crypto: Handle virtio_device_ready() failure
drm/virtio: Handle virtio_device_ready() failure
virtio-iommu: Handle virtio_device_ready() failure
virtio-net: Handle virtio_device_ready() failure
rpmsg: virtio: Handle virtio_device_ready() failure
virtio_scsi: Handle virtio_device_ready() failure
virtio: Handle virtio_device_ready() failure
virtio-balloon: Handle virtio_device_ready() failure
virtio-input: Handle virtio_device_ready() failure
virtio-mem: Handle virtio_device_ready() failure
virtiofs: Handle virtio_device_ready() failure
9p/trans_virtio: Handle virtio_device_ready() failure
drivers/block/virtio_blk.c | 13 +++++++++++--
drivers/char/virtio_console.c | 14 ++++++++++++--
drivers/crypto/virtio/virtio_crypto_core.c | 8 ++++++--
drivers/gpu/drm/virtio/virtgpu_kms.c | 8 +++++++-
drivers/iommu/virtio-iommu.c | 4 +++-
drivers/net/virtio_net.c | 19 +++++++++++++-----
drivers/rpmsg/virtio_rpmsg_bus.c | 31 ++++++++++++++++++------------
drivers/scsi/virtio_scsi.c | 13 ++++++++++---
drivers/virtio/virtio.c | 9 +++++++--
drivers/virtio/virtio_balloon.c | 13 +++++++++++--
drivers/virtio/virtio_input.c | 11 +++++++++--
drivers/virtio/virtio_mem.c | 6 +++++-
fs/fuse/virtio_fs.c | 4 +++-
include/linux/virtio_config.h | 8 ++++++--
net/9p/trans_virtio.c | 6 +++++-
15 files changed, 128 insertions(+), 39 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 01/15] virtio_config: Add return value to virtio_device_ready()
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 02/15] virtio-blk: Handle virtio_device_ready() failure Xie Yongji
` (13 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
We might get invalid status from untrusted device. Let's remove
BUG_ON and add return value to virtio_device_ready() to handle
this case gracefully.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
include/linux/virtio_config.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 8519b3ae5d52..0e61cd89ac1d 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -225,12 +225,16 @@ int virtio_find_vqs_ctx(struct virtio_device *vdev, unsigned nvqs,
* Note: vqs are enabled automatically after probe returns.
*/
static inline
-void virtio_device_ready(struct virtio_device *dev)
+int virtio_device_ready(struct virtio_device *dev)
{
unsigned status = dev->config->get_status(dev);
- BUG_ON(status & VIRTIO_CONFIG_S_DRIVER_OK);
+ if (status & VIRTIO_CONFIG_S_DRIVER_OK)
+ return -EINVAL;
+
dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
+
+ return 0;
}
static inline
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 02/15] virtio-blk: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 01/15] virtio_config: Add return value to virtio_device_ready() Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 03/15] virtio_console: " Xie Yongji
` (12 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case in both probe
and resume paths.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/block/virtio_blk.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 255adb7a768c..ebb4d3fe803f 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -884,11 +884,15 @@ static int virtblk_probe(struct virtio_device *vdev)
}
virtblk_update_capacity(vblk, false);
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err)
+ goto out_cleanup_queue;
device_add_disk(&vdev->dev, vblk->disk, virtblk_attr_groups);
return 0;
+out_cleanup_queue:
+ blk_cleanup_queue(vblk->disk->queue);
out_free_tags:
blk_mq_free_tag_set(&vblk->tag_set);
out_put_disk:
@@ -961,7 +965,12 @@ static int virtblk_restore(struct virtio_device *vdev)
if (ret)
return ret;
- virtio_device_ready(vdev);
+ ret = virtio_device_ready(vdev);
+ if (ret) {
+ vdev->config->del_vqs(vdev);
+ kfree(vblk->vqs);
+ return ret;
+ }
blk_mq_unquiesce_queue(vblk->disk->queue);
return 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 03/15] virtio_console: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 01/15] virtio_config: Add return value to virtio_device_ready() Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 02/15] virtio-blk: Handle virtio_device_ready() failure Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 04/15] virtio_crypto: " Xie Yongji
` (11 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case in both probe
and resume paths.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/char/virtio_console.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index b85abe1eb2d1..1c40ca6d76ba 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2048,7 +2048,11 @@ static int virtcons_probe(struct virtio_device *vdev)
INIT_LIST_HEAD(&portdev->ports);
INIT_LIST_HEAD(&portdev->list);
- virtio_device_ready(portdev->vdev);
+ err = virtio_device_ready(portdev->vdev);
+ if (err) {
+ dev_err(&vdev->dev, "Error %d enable virtio device\n", err);
+ goto free_vqs;
+ }
INIT_WORK(&portdev->config_work, &config_work_handler);
INIT_WORK(&portdev->control_work, &control_work_handler);
@@ -2100,6 +2104,8 @@ static int virtcons_probe(struct virtio_device *vdev)
return 0;
+free_vqs:
+ remove_vqs(portdev);
free_chrdev:
unregister_chrdev(portdev->chr_major, "virtio-portsdev");
free:
@@ -2178,7 +2184,11 @@ static int virtcons_restore(struct virtio_device *vdev)
if (ret)
return ret;
- virtio_device_ready(portdev->vdev);
+ ret = virtio_device_ready(portdev->vdev);
+ if (ret) {
+ remove_vqs(portdev);
+ return ret;
+ }
if (use_multiport(portdev))
fill_queue(portdev->c_ivq, &portdev->c_ivq_lock);
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 04/15] virtio_crypto: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (2 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 03/15] virtio_console: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 05/15] drm/virtio: " Xie Yongji
` (10 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case in both probe
and resume paths.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/crypto/virtio/virtio_crypto_core.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index 080955a1dd9c..18f7ffc37738 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -393,7 +393,9 @@ static int virtcrypto_probe(struct virtio_device *vdev)
if (err)
goto free_vqs;
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err)
+ goto free_engines;
err = virtcrypto_update_status(vcrypto);
if (err)
@@ -479,7 +481,9 @@ static int virtcrypto_restore(struct virtio_device *vdev)
if (err)
goto free_vqs;
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err)
+ goto free_engines;
err = virtcrypto_dev_start(vcrypto);
if (err) {
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 05/15] drm/virtio: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (3 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 04/15] virtio_crypto: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 06/15] virtio-iommu: " Xie Yongji
` (9 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case on probe.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/gpu/drm/virtio/virtgpu_kms.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index f3379059f324..eedbb4684db5 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -217,7 +217,11 @@ int virtio_gpu_init(struct drm_device *dev)
goto err_scanouts;
}
- virtio_device_ready(vgdev->vdev);
+ ret = virtio_device_ready(vgdev->vdev);
+ if (ret) {
+ DRM_ERROR("failed to enable virtio device\n");
+ goto err_device_ready;
+ }
if (num_capsets)
virtio_gpu_get_capsets(vgdev, num_capsets);
@@ -229,6 +233,8 @@ int virtio_gpu_init(struct drm_device *dev)
5 * HZ);
return 0;
+err_device_ready:
+ virtio_gpu_modeset_fini(vgdev);
err_scanouts:
virtio_gpu_free_vbufs(vgdev);
err_vbufs:
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 06/15] virtio-iommu: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (4 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 05/15] drm/virtio: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 07/15] virtio-net: " Xie Yongji
` (8 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case on probe.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/iommu/virtio-iommu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 2bfdd5734844..0d4840c5841a 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -1053,7 +1053,9 @@ static int viommu_probe(struct virtio_device *vdev)
viommu_ops.pgsize_bitmap = viommu->pgsize_bitmap;
- virtio_device_ready(vdev);
+ ret = virtio_device_ready(vdev);
+ if (ret)
+ goto err_free_vqs;
/* Populate the event queue with buffers */
ret = viommu_fill_evtq(viommu);
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 07/15] virtio-net: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (5 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 06/15] virtio-iommu: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` Xie Yongji
` (7 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case in both probe
and resume paths.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/net/virtio_net.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 01e54e99cae6..c4711e23af88 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2341,6 +2341,7 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
}
static int init_vqs(struct virtnet_info *vi);
+static void virtnet_del_vqs(struct virtnet_info *vi);
static int virtnet_restore_up(struct virtio_device *vdev)
{
@@ -2351,7 +2352,11 @@ static int virtnet_restore_up(struct virtio_device *vdev)
if (err)
return err;
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err) {
+ virtnet_del_vqs(vi);
+ return err;
+ }
if (netif_running(vi->dev)) {
for (i = 0; i < vi->curr_queue_pairs; i++)
@@ -3148,12 +3153,16 @@ static int virtnet_probe(struct virtio_device *vdev)
goto free_failover;
}
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err) {
+ pr_debug("virtio_net: enable virtio device failed\n");
+ goto free_unregister_netdev;
+ }
err = virtnet_cpu_notif_add(vi);
if (err) {
pr_debug("virtio_net: registering cpu notifier failed\n");
- goto free_unregister_netdev;
+ goto reset_virtio_device;
}
virtnet_set_queues(vi, vi->curr_queue_pairs);
@@ -3179,9 +3188,9 @@ static int virtnet_probe(struct virtio_device *vdev)
return 0;
-free_unregister_netdev:
+reset_virtio_device:
vi->vdev->config->reset(vdev);
-
+free_unregister_netdev:
unregister_netdev(dev);
free_failover:
net_failover_destroy(vi->failover);
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 07/15] virtio-net: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (6 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 07/15] virtio-net: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 08/15] rpmsg: virtio: " Xie Yongji
` (6 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case in both probe
and resume paths.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/net/virtio_net.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 01e54e99cae6..c4711e23af88 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2341,6 +2341,7 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
}
static int init_vqs(struct virtnet_info *vi);
+static void virtnet_del_vqs(struct virtnet_info *vi);
static int virtnet_restore_up(struct virtio_device *vdev)
{
@@ -2351,7 +2352,11 @@ static int virtnet_restore_up(struct virtio_device *vdev)
if (err)
return err;
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err) {
+ virtnet_del_vqs(vi);
+ return err;
+ }
if (netif_running(vi->dev)) {
for (i = 0; i < vi->curr_queue_pairs; i++)
@@ -3148,12 +3153,16 @@ static int virtnet_probe(struct virtio_device *vdev)
goto free_failover;
}
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err) {
+ pr_debug("virtio_net: enable virtio device failed\n");
+ goto free_unregister_netdev;
+ }
err = virtnet_cpu_notif_add(vi);
if (err) {
pr_debug("virtio_net: registering cpu notifier failed\n");
- goto free_unregister_netdev;
+ goto reset_virtio_device;
}
virtnet_set_queues(vi, vi->curr_queue_pairs);
@@ -3179,9 +3188,9 @@ static int virtnet_probe(struct virtio_device *vdev)
return 0;
-free_unregister_netdev:
+reset_virtio_device:
vi->vdev->config->reset(vdev);
-
+free_unregister_netdev:
unregister_netdev(dev);
free_failover:
net_failover_destroy(vi->failover);
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 08/15] rpmsg: virtio: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (7 preceding siblings ...)
2021-05-17 9:34 ` Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 10/15] " Xie Yongji
` (5 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case on probe.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/rpmsg/virtio_rpmsg_bus.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 10df8bc0c3e1..6a8eb8c74bca 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -812,6 +812,19 @@ static void rpmsg_xmit_done(struct virtqueue *svq)
wake_up_interruptible(&vrp->sendq);
}
+static int rpmsg_remove_device(struct device *dev, void *data)
+{
+ device_unregister(dev);
+
+ return 0;
+}
+
+static void virtio_rpmsg_remove_device(struct virtio_device *vdev)
+{
+ if (device_for_each_child(&vdev->dev, NULL, rpmsg_remove_device))
+ dev_warn(&vdev->dev, "can't remove rpmsg device: %d\n", ret);
+}
+
static int rpmsg_probe(struct virtio_device *vdev)
{
vq_callback_t *vq_cbs[] = { rpmsg_recv_done, rpmsg_xmit_done };
@@ -924,7 +937,9 @@ static int rpmsg_probe(struct virtio_device *vdev)
notify = virtqueue_kick_prepare(vrp->rvq);
/* From this point on, we can notify and get callbacks. */
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err)
+ goto remove_rpmsg_dev;
/* tell the remote processor it can start sending messages */
/*
@@ -937,7 +952,8 @@ static int rpmsg_probe(struct virtio_device *vdev)
dev_info(&vdev->dev, "rpmsg host is online\n");
return 0;
-
+remove_rpmsg_dev:
+ virtio_rpmsg_remove_device(vdev);
free_coherent:
kfree(vch);
dma_free_coherent(vdev->dev.parent, total_buf_space,
@@ -949,13 +965,6 @@ static int rpmsg_probe(struct virtio_device *vdev)
return err;
}
-static int rpmsg_remove_device(struct device *dev, void *data)
-{
- device_unregister(dev);
-
- return 0;
-}
-
static void rpmsg_remove(struct virtio_device *vdev)
{
struct virtproc_info *vrp = vdev->priv;
@@ -964,9 +973,7 @@ static void rpmsg_remove(struct virtio_device *vdev)
vdev->config->reset(vdev);
- ret = device_for_each_child(&vdev->dev, NULL, rpmsg_remove_device);
- if (ret)
- dev_warn(&vdev->dev, "can't remove rpmsg device: %d\n", ret);
+ virtio_rpmsg_remove_device(vdev);
idr_destroy(&vrp->endpoints);
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 10/15] virtio: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (8 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 08/15] rpmsg: virtio: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 11/15] virtio-balloon: " Xie Yongji
` (4 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case in virtio_dev_probe().
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/virtio/virtio.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 4b15c00c0a0a..ff16e1cce2e7 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -263,8 +263,11 @@ static int virtio_dev_probe(struct device *_d)
goto err;
/* If probe didn't do it, mark device DRIVER_OK ourselves. */
- if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
- virtio_device_ready(dev);
+ if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK)) {
+ err = virtio_device_ready(dev);
+ if (err)
+ goto err_ready;
+ }
if (drv->scan)
drv->scan(dev);
@@ -272,6 +275,8 @@ static int virtio_dev_probe(struct device *_d)
virtio_config_enable(dev);
return 0;
+err_ready:
+ drv->remove(dev);
err:
virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
return err;
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 11/15] virtio-balloon: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (9 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 10/15] " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 12/15] virtio-input: " Xie Yongji
` (3 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case in both probe
and resume paths.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/virtio/virtio_balloon.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7565970c3901..4f183dd901b3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -997,12 +997,17 @@ static int virtballoon_probe(struct virtio_device *vdev)
goto out_unregister_oom;
}
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err)
+ goto out_unregister_page_report;
if (towards_target(vb))
virtballoon_changed(vdev);
return 0;
+out_unregister_page_report:
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
+ page_reporting_unregister(&vb->pr_dev_info);
out_unregister_oom:
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
unregister_oom_notifier(&vb->oom_nb);
@@ -1096,7 +1101,11 @@ static int virtballoon_restore(struct virtio_device *vdev)
if (ret)
return ret;
- virtio_device_ready(vdev);
+ ret = virtio_device_ready(vdev);
+ if (ret) {
+ vdev->config->del_vqs(vdev);
+ return ret;
+ }
if (towards_target(vb))
virtballoon_changed(vdev);
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 12/15] virtio-input: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (10 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 11/15] virtio-balloon: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 13/15] virtio-mem: " Xie Yongji
` (2 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case in both probe
and resume paths.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/virtio/virtio_input.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index f83f8e556fba..a94940efad7a 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -312,7 +312,10 @@ static int virtinput_probe(struct virtio_device *vdev)
}
}
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err)
+ goto err_mt_init_slots;
+
vi->ready = true;
err = input_register_device(vi->idev);
if (err)
@@ -375,7 +378,11 @@ static int virtinput_restore(struct virtio_device *vdev)
if (err)
return err;
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err) {
+ vdev->config->del_vqs(vdev);
+ return err;
+ }
vi->ready = true;
virtinput_fill_evt(vi);
return 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 13/15] virtio-mem: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (11 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 12/15] virtio-input: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:48 ` David Hildenbrand
2021-05-17 9:34 ` [RFC PATCH 14/15] virtiofs: " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 15/15] 9p/trans_virtio: " Xie Yongji
14 siblings, 1 reply; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case on probe.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/virtio/virtio_mem.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 32a8e359a5c3..1148c392ff94 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -2602,13 +2602,17 @@ static int virtio_mem_probe(struct virtio_device *vdev)
if (rc)
goto out_unreg_mem;
- virtio_device_ready(vdev);
+ rc = virtio_device_ready(vdev);
+ if (rc)
+ goto out_unreg_device;
/* trigger a config update to start processing the requested_size */
atomic_set(&vm->config_changed, 1);
queue_work(system_freezable_wq, &vm->wq);
return 0;
+out_unreg_device:
+ unregister_virtio_mem_device(vm);
out_unreg_mem:
unregister_memory_notifier(&vm->memory_notifier);
out_del_resource:
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 14/15] virtiofs: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (12 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 13/15] virtio-mem: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 15/15] 9p/trans_virtio: " Xie Yongji
14 siblings, 0 replies; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case on probe.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
fs/fuse/virtio_fs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index e61c94eaa20f..ade0dc42ebfd 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -883,7 +883,9 @@ static int virtio_fs_probe(struct virtio_device *vdev)
/* Bring the device online in case the filesystem is mounted and
* requests need to be sent before we return.
*/
- virtio_device_ready(vdev);
+ ret = virtio_device_ready(vdev);
+ if (ret < 0)
+ goto out_vqs;
ret = virtio_fs_add_instance(fs);
if (ret < 0)
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 15/15] 9p/trans_virtio: Handle virtio_device_ready() failure
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
` (13 preceding siblings ...)
2021-05-17 9:34 ` [RFC PATCH 14/15] virtiofs: " Xie Yongji
@ 2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:53 ` asmadeus
14 siblings, 1 reply; 19+ messages in thread
From: Xie Yongji @ 2021-05-17 9:34 UTC (permalink / raw)
To: mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, david, vgoyal, miklos, lucho, asmadeus,
virtualization, linux-kernel
Now virtio_device_ready() will return error if we get
invalid status. Let's handle this case on probe.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
net/9p/trans_virtio.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 0960ed1ad7ac..6013d3761b76 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -617,7 +617,9 @@ static int p9_virtio_probe(struct virtio_device *vdev)
/* Ceiling limit to avoid denial of service attacks */
chan->p9_max_pages = nr_free_buffer_pages()/4;
- virtio_device_ready(vdev);
+ err = virtio_device_ready(vdev);
+ if (err)
+ goto out_free_vc_wq;
mutex_lock(&virtio_9p_lock);
list_add_tail(&chan->chan_list, &virtio_chan_list);
@@ -628,6 +630,8 @@ static int p9_virtio_probe(struct virtio_device *vdev)
return 0;
+out_free_vc_wq:
+ kfree(chan->vc_wq);
out_remove_file:
sysfs_remove_file(&(vdev->dev.kobj), &dev_attr_mount_tag.attr);
out_free_tag:
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 13/15] virtio-mem: Handle virtio_device_ready() failure
2021-05-17 9:34 ` [RFC PATCH 13/15] virtio-mem: " Xie Yongji
@ 2021-05-17 9:48 ` David Hildenbrand
2021-05-17 10:08 ` Yongji Xie
0 siblings, 1 reply; 19+ messages in thread
From: David Hildenbrand @ 2021-05-17 9:48 UTC (permalink / raw)
To: Xie Yongji, mst, jasowang, stefanha
Cc: amit, arei.gonglei, airlied, kraxel, jean-philippe, ohad,
bjorn.andersson, vgoyal, miklos, lucho, asmadeus, virtualization,
linux-kernel
On 17.05.21 11:34, Xie Yongji wrote:
> Now virtio_device_ready() will return error if we get
> invalid status. Let's handle this case on probe.
>
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> ---
> drivers/virtio/virtio_mem.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 32a8e359a5c3..1148c392ff94 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -2602,13 +2602,17 @@ static int virtio_mem_probe(struct virtio_device *vdev)
> if (rc)
> goto out_unreg_mem;
>
> - virtio_device_ready(vdev);
> + rc = virtio_device_ready(vdev);
> + if (rc)
> + goto out_unreg_device;
>
> /* trigger a config update to start processing the requested_size */
> atomic_set(&vm->config_changed, 1);
> queue_work(system_freezable_wq, &vm->wq);
>
> return 0;
> +out_unreg_device:
> + unregister_virtio_mem_device(vm);
> out_unreg_mem:
> unregister_memory_notifier(&vm->memory_notifier);
> out_del_resource:
>
I assume this will really be a corner case to hit, right?
Failing after essentially being done initializing looks sub-optimal, anyhow:
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 15/15] 9p/trans_virtio: Handle virtio_device_ready() failure
2021-05-17 9:34 ` [RFC PATCH 15/15] 9p/trans_virtio: " Xie Yongji
@ 2021-05-17 9:53 ` asmadeus
0 siblings, 0 replies; 19+ messages in thread
From: asmadeus @ 2021-05-17 9:53 UTC (permalink / raw)
To: Xie Yongji
Cc: mst, jasowang, stefanha, amit, arei.gonglei, airlied, kraxel,
jean-philippe, ohad, bjorn.andersson, david, vgoyal, miklos,
lucho, virtualization, linux-kernel
Xie Yongji wrote on Mon, May 17, 2021 at 05:34:28PM +0800:
> Now virtio_device_ready() will return error if we get
> invalid status. Let's handle this case on probe.
The change itself looks good to me
It's going to be a pain to apply though because it depends on
https://lkml.kernel.org/r/20210517083557.172-1-xieyongji@bytedance.com
you just sent, so this won't apply to virtio maintainers on one hand,
and I can't take just this patch because it depends on the first patch
in the patchset (well if it weren't RFC)...
I guess sending the other patch to Linus fast-ish so it can go in virtio
tree would be the best way forward? other maintainers please advise, I'm
bad at this.
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Acked-by: Dominique Martinet <asmadeus@codewreck.org>
--
Dominique
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: [RFC PATCH 13/15] virtio-mem: Handle virtio_device_ready() failure
2021-05-17 9:48 ` David Hildenbrand
@ 2021-05-17 10:08 ` Yongji Xie
0 siblings, 0 replies; 19+ messages in thread
From: Yongji Xie @ 2021-05-17 10:08 UTC (permalink / raw)
To: David Hildenbrand
Cc: Michael S. Tsirkin, Jason Wang, Stefan Hajnoczi, amit,
arei.gonglei, airlied, kraxel, jean-philippe, Ohad Ben Cohen,
bjorn.andersson, vgoyal, miklos, lucho, asmadeus, virtualization,
linux-kernel
On Mon, May 17, 2021 at 5:48 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 17.05.21 11:34, Xie Yongji wrote:
> > Now virtio_device_ready() will return error if we get
> > invalid status. Let's handle this case on probe.
> >
> > Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> > ---
> > drivers/virtio/virtio_mem.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> > index 32a8e359a5c3..1148c392ff94 100644
> > --- a/drivers/virtio/virtio_mem.c
> > +++ b/drivers/virtio/virtio_mem.c
> > @@ -2602,13 +2602,17 @@ static int virtio_mem_probe(struct virtio_device *vdev)
> > if (rc)
> > goto out_unreg_mem;
> >
> > - virtio_device_ready(vdev);
> > + rc = virtio_device_ready(vdev);
> > + if (rc)
> > + goto out_unreg_device;
> >
> > /* trigger a config update to start processing the requested_size */
> > atomic_set(&vm->config_changed, 1);
> > queue_work(system_freezable_wq, &vm->wq);
> >
> > return 0;
> > +out_unreg_device:
> > + unregister_virtio_mem_device(vm);
> > out_unreg_mem:
> > unregister_memory_notifier(&vm->memory_notifier);
> > out_del_resource:
> >
>
> I assume this will really be a corner case to hit, right?
>
Yes, it should almost never happen if the device is trusted.
Thanks,
Yongji
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-05-17 10:08 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17 9:34 [RFC PATCH 00/15] Handle virtio_device_ready() failure Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 01/15] virtio_config: Add return value to virtio_device_ready() Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 02/15] virtio-blk: Handle virtio_device_ready() failure Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 03/15] virtio_console: " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 04/15] virtio_crypto: " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 05/15] drm/virtio: " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 06/15] virtio-iommu: " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 07/15] virtio-net: " Xie Yongji
2021-05-17 9:34 ` Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 08/15] rpmsg: virtio: " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 10/15] " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 11/15] virtio-balloon: " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 12/15] virtio-input: " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 13/15] virtio-mem: " Xie Yongji
2021-05-17 9:48 ` David Hildenbrand
2021-05-17 10:08 ` Yongji Xie
2021-05-17 9:34 ` [RFC PATCH 14/15] virtiofs: " Xie Yongji
2021-05-17 9:34 ` [RFC PATCH 15/15] 9p/trans_virtio: " Xie Yongji
2021-05-17 9:53 ` asmadeus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).