I'm experiencing a segmentation fault in lkvm where it may crash after
powering off a guest machine that uses a virtio network device.
The crash is hard to reproduce, because looks like it only happens
when the guest machine is powering off while extra virtio threads is
doing some work,
when it happens lkvm crashes in the function virtio_net_rx_thread
while attempting to read invalid guest physical memory,
because guest physical memory was unmapped.
I've isolated the problem and looks like when lkvm exits it unmaps the
guest memory while virtio device extra threads may still be executing.
I noticed most virtio devices are not executing pthread_cancel +
pthread_join to synchronize extra threads when exiting,
to make sure this happens I added explicit calls to the virtio device
exit function to all virtio devices,
which should cancel and join all threads before unmapping guest
physical memory, fixing the crash for me.
Below I'm attaching a patch to fix the issue, feel free to apply or
fix the issue some other way.
Signed-off-by: Eduardo Bart <edub4rt@xxxxxxxxx>
---
include/kvm/virtio-9p.h | 1 +
virtio/9p.c | 14 ++++++++++++++
virtio/balloon.c | 11 +++++++++++
virtio/blk.c | 1 +
virtio/console.c | 3 +++
virtio/net.c | 4 ++++
virtio/rng.c | 8 ++++++++
7 files changed, 42 insertions(+)
diff --git a/include/kvm/virtio-9p.h b/include/kvm/virtio-9p.h
index 1dffc95..09f7e46 100644
--- a/include/kvm/virtio-9p.h
+++ b/include/kvm/virtio-9p.h
@@ -70,6 +70,7 @@ int virtio_9p_rootdir_parser(const struct option
*opt, const char *arg, int unse
int virtio_9p_img_name_parser(const struct option *opt, const char
*arg, int unset);
int virtio_9p__register(struct kvm *kvm, const char *root, const char
*tag_name);
int virtio_9p__init(struct kvm *kvm);
+int virtio_9p__exit(struct kvm *kvm);
int virtio_p9_pdu_readf(struct p9_pdu *pdu, const char *fmt, ...);
int virtio_p9_pdu_writef(struct p9_pdu *pdu, const char *fmt, ...);
diff --git a/virtio/9p.c b/virtio/9p.c
index 513164e..f536d9e 100644
--- a/virtio/9p.c
+++ b/virtio/9p.c
@@ -1562,6 +1562,20 @@ int virtio_9p__init(struct kvm *kvm)
}
virtio_dev_init(virtio_9p__init);
+int virtio_9p__exit(struct kvm *kvm)
+{
+ struct p9_dev *p9dev, *tmp;
+
+ list_for_each_entry_safe(p9dev, tmp, &devs, list) {
+ list_del(&p9dev->list);
+ p9dev->vdev.ops->exit(kvm, &p9dev->vdev);
+ free(p9dev);
+ }
+
+ return 0;
+}
+virtio_dev_exit(virtio_9p__exit);
+
int virtio_9p__register(struct kvm *kvm, const char *root, const char
*tag_name)
{
struct p9_dev *p9dev;
diff --git a/virtio/balloon.c b/virtio/balloon.c
index 01d1982..a36e50e 100644
--- a/virtio/balloon.c
+++ b/virtio/balloon.c
@@ -221,6 +221,13 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq)
return 0;
}
+static void exit_vq(struct kvm *kvm, void *dev, u32 vq)
+{
+ struct bln_dev *bdev = dev;
+
+ thread_pool__cancel_job(&bdev->jobs[vq]);
+}
+
static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
{
struct bln_dev *bdev = dev;
@@ -258,6 +265,7 @@ struct virtio_ops bln_dev_virtio_ops = {
.get_config_size = get_config_size,
.get_host_features = get_host_features,
.init_vq = init_vq,
+ .exit_vq = exit_vq,
.notify_vq = notify_vq,
.get_vq = get_vq,
.get_size_vq = get_size_vq,
@@ -293,6 +301,9 @@ virtio_dev_init(virtio_bln__init);
int virtio_bln__exit(struct kvm *kvm)
{
+ if (bdev.vdev.ops)
+ bdev.vdev.ops->exit(kvm, &bdev.vdev);
+
return 0;
}
virtio_dev_exit(virtio_bln__exit);
diff --git a/virtio/blk.c b/virtio/blk.c
index a58c745..e34723a 100644
--- a/virtio/blk.c
+++ b/virtio/blk.c
@@ -345,6 +345,7 @@ static int virtio_blk__init_one(struct kvm *kvm,
struct disk_image *disk)
static int virtio_blk__exit_one(struct kvm *kvm, struct blk_dev *bdev)
{
list_del(&bdev->list);
+ bdev->vdev.ops->exit(kvm, &bdev->vdev);
free(bdev);
return 0;
diff --git a/virtio/console.c b/virtio/console.c
index ebfbaf0..5a71bbc 100644
--- a/virtio/console.c
+++ b/virtio/console.c
@@ -243,6 +243,9 @@ virtio_dev_init(virtio_console__init);
int virtio_console__exit(struct kvm *kvm)
{
+ if (cdev.vdev.ops)
+ cdev.vdev.ops->exit(kvm, &cdev.vdev);
+
return 0;
}
virtio_dev_exit(virtio_console__exit);
diff --git a/virtio/net.c b/virtio/net.c
index f09dd0a..dc6d89d 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -969,10 +969,14 @@ int virtio_net__exit(struct kvm *kvm)
if (ndev->mode == NET_MODE_TAP &&
strcmp(params->downscript, "none"))
virtio_net_exec_script(params->downscript, ndev->tap_name);
+ if (ndev->mode != NET_MODE_TAP)
+ uip_exit(&ndev->info);
list_del(&ndev->list);
+ ndev->vdev.ops->exit(kvm, &ndev->vdev);
free(ndev);
}
+
return 0;
}
virtio_dev_exit(virtio_net__exit);
diff --git a/virtio/rng.c b/virtio/rng.c
index 6b36655..ebdb455 100644
--- a/virtio/rng.c
+++ b/virtio/rng.c
@@ -122,6 +122,13 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq)
return 0;
}
+static void exit_vq(struct kvm *kvm, void *dev, u32 vq)
+{
+ struct rng_dev *rdev = dev;
+
+ thread_pool__cancel_job(&rdev->jobs[vq].job_id);
+}
+
static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
{
struct rng_dev *rdev = dev;
@@ -159,6 +166,7 @@ static struct virtio_ops rng_dev_virtio_ops = {
.get_config_size = get_config_size,
.get_host_features = get_host_features,
.init_vq = init_vq,
+ .exit_vq = exit_vq,
.notify_vq = notify_vq,
.get_vq = get_vq,
.get_size_vq = get_size_vq,
--
2.42.0
