From 04e5b1fb01834a602acaae2276b67a783a8c6159 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Wed, 18 Sep 2019 21:24:13 +0200 Subject: um: virtio: Remove device on disconnect If the connection drops, just remove the device, we don't try to recover from this right now. Signed-off-by: Johannes Berg Signed-off-by: Richard Weinberger --- arch/um/drivers/virtio_uml.c | 64 +++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 19 deletions(-) (limited to 'arch/um/drivers/virtio_uml.c') diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c index fc8c52cff5aa..ca3067302c15 100644 --- a/arch/um/drivers/virtio_uml.c +++ b/arch/um/drivers/virtio_uml.c @@ -42,6 +42,13 @@ #define to_virtio_uml_device(_vdev) \ container_of(_vdev, struct virtio_uml_device, vdev) +struct virtio_uml_platform_data { + u32 virtio_device_id; + const char *socket_path; + struct work_struct conn_broken_wk; + struct platform_device *pdev; +}; + struct virtio_uml_device { struct virtio_device vdev; struct platform_device *pdev; @@ -50,6 +57,7 @@ struct virtio_uml_device { u64 features; u64 protocol_features; u8 status; + u8 registered:1; }; struct virtio_uml_vq_info { @@ -107,12 +115,21 @@ static int vhost_user_recv_header(int fd, struct vhost_user_msg *msg) return full_read(fd, msg, sizeof(msg->header)); } -static int vhost_user_recv(int fd, struct vhost_user_msg *msg, +static int vhost_user_recv(struct virtio_uml_device *vu_dev, + int fd, struct vhost_user_msg *msg, size_t max_payload_size) { size_t size; int rc = vhost_user_recv_header(fd, msg); + if (rc == -ECONNRESET && vu_dev->registered) { + struct virtio_uml_platform_data *pdata; + + pdata = vu_dev->pdev->dev.platform_data; + + virtio_break_device(&vu_dev->vdev); + schedule_work(&pdata->conn_broken_wk); + } if (rc) return rc; size = msg->header.size; @@ -125,7 +142,7 @@ static int vhost_user_recv_resp(struct virtio_uml_device *vu_dev, struct vhost_user_msg *msg, size_t max_payload_size) { - int rc = vhost_user_recv(vu_dev->sock, msg, max_payload_size); + int rc = vhost_user_recv(vu_dev, vu_dev->sock, msg, max_payload_size); if (rc) return rc; @@ -155,7 +172,7 @@ static int vhost_user_recv_req(struct virtio_uml_device *vu_dev, struct vhost_user_msg *msg, size_t max_payload_size) { - int rc = vhost_user_recv(vu_dev->req_fd, msg, max_payload_size); + int rc = vhost_user_recv(vu_dev, vu_dev->req_fd, msg, max_payload_size); if (rc) return rc; @@ -963,11 +980,6 @@ static void virtio_uml_release_dev(struct device *d) /* Platform device */ -struct virtio_uml_platform_data { - u32 virtio_device_id; - const char *socket_path; -}; - static int virtio_uml_probe(struct platform_device *pdev) { struct virtio_uml_platform_data *pdata = pdev->dev.platform_data; @@ -1005,6 +1017,7 @@ static int virtio_uml_probe(struct platform_device *pdev) rc = register_virtio_device(&vu_dev->vdev); if (rc) put_device(&vu_dev->vdev.dev); + vu_dev->registered = 1; return rc; error_init: @@ -1034,13 +1047,31 @@ static struct device vu_cmdline_parent = { static bool vu_cmdline_parent_registered; static int vu_cmdline_id; +static int vu_unregister_cmdline_device(struct device *dev, void *data) +{ + struct platform_device *pdev = to_platform_device(dev); + struct virtio_uml_platform_data *pdata = pdev->dev.platform_data; + + kfree(pdata->socket_path); + platform_device_unregister(pdev); + return 0; +} + +static void vu_conn_broken(struct work_struct *wk) +{ + struct virtio_uml_platform_data *pdata; + + pdata = container_of(wk, struct virtio_uml_platform_data, conn_broken_wk); + vu_unregister_cmdline_device(&pdata->pdev->dev, NULL); +} + static int vu_cmdline_set(const char *device, const struct kernel_param *kp) { const char *ids = strchr(device, ':'); unsigned int virtio_device_id; int processed, consumed, err; char *socket_path; - struct virtio_uml_platform_data pdata; + struct virtio_uml_platform_data pdata, *ppdata; struct platform_device *pdev; if (!ids || ids == device) @@ -1079,6 +1110,11 @@ static int vu_cmdline_set(const char *device, const struct kernel_param *kp) err = PTR_ERR_OR_ZERO(pdev); if (err) goto free; + + ppdata = pdev->dev.platform_data; + ppdata->pdev = pdev; + INIT_WORK(&ppdata->conn_broken_wk, vu_conn_broken); + return 0; free: @@ -1121,16 +1157,6 @@ __uml_help(vu_cmdline_param_ops, ); -static int vu_unregister_cmdline_device(struct device *dev, void *data) -{ - struct platform_device *pdev = to_platform_device(dev); - struct virtio_uml_platform_data *pdata = pdev->dev.platform_data; - - kfree(pdata->socket_path); - platform_device_unregister(pdev); - return 0; -} - static void vu_unregister_cmdline_devices(void) { if (vu_cmdline_parent_registered) { -- cgit v1.2.3 From 7e60746005573a06149cdee7acedf428906f3a59 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Tue, 24 Sep 2019 09:21:17 +0200 Subject: um: virtio: Keep reading on -EAGAIN When we get an interrupt from the socket getting readable, and start reading, there's a possibility for a race. This depends on the implementation of the device, but e.g. with qemu's libvhost-user, we can see: device virtio_uml --------------------------------------- write header get interrupt read header read body -> returns -EAGAIN write body The -EAGAIN return is because the socket is non-blocking, and then this leads us to abandon this message. In fact, we've already read the header, so when the get another signal/interrupt for the body, we again read it as though it's a new message header, and also abandon it for the same reason (wrong size etc.) This essentially breaks things, and if that message was one that required a response, it leads to a deadlock as the device is waiting for the response but we'll never reply. Fix this by spinning on -EAGAIN as well when we read the message body. We need to handle -EAGAIN as "no message" while reading the header, since we share an interrupt. Note that this situation is highly unlikely to occur in normal usage, since there will be very few messages and only in the startup phase. With the inband call feature this does tend to happen (eventually) though. Signed-off-by: Johannes Berg Signed-off-by: Richard Weinberger --- arch/um/drivers/virtio_uml.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'arch/um/drivers/virtio_uml.c') diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c index ca3067302c15..76b97c2de9a8 100644 --- a/arch/um/drivers/virtio_uml.c +++ b/arch/um/drivers/virtio_uml.c @@ -91,7 +91,7 @@ static int full_sendmsg_fds(int fd, const void *buf, unsigned int len, return 0; } -static int full_read(int fd, void *buf, int len) +static int full_read(int fd, void *buf, int len, bool abortable) { int rc; @@ -101,7 +101,7 @@ static int full_read(int fd, void *buf, int len) buf += rc; len -= rc; } - } while (len && (rc > 0 || rc == -EINTR)); + } while (len && (rc > 0 || rc == -EINTR || (!abortable && rc == -EAGAIN))); if (rc < 0) return rc; @@ -112,7 +112,7 @@ static int full_read(int fd, void *buf, int len) static int vhost_user_recv_header(int fd, struct vhost_user_msg *msg) { - return full_read(fd, msg, sizeof(msg->header)); + return full_read(fd, msg, sizeof(msg->header), true); } static int vhost_user_recv(struct virtio_uml_device *vu_dev, @@ -135,7 +135,7 @@ static int vhost_user_recv(struct virtio_uml_device *vu_dev, size = msg->header.size; if (size > max_payload_size) return -EPROTO; - return full_read(fd, &msg->payload, size); + return full_read(fd, &msg->payload, size, false); } static int vhost_user_recv_resp(struct virtio_uml_device *vu_dev, -- cgit v1.2.3 From bf9f80cf0ccab5f346f7d3cdc445da8fcfe6ce34 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Tue, 8 Oct 2019 17:43:21 +0200 Subject: um: virtio_uml: Disallow modular build This driver *can* be a module, but then its parameters (socket path) are untrusted data from inside the VM, and that isn't allowed. Allow the code to only be built-in to avoid that. Fixes: 5d38f324993f ("um: drivers: Add virtio vhost-user driver") Signed-off-by: Johannes Berg Acked-by: Anton Ivanov Signed-off-by: Richard Weinberger --- arch/um/drivers/Kconfig | 2 +- arch/um/drivers/virtio_uml.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'arch/um/drivers/virtio_uml.c') diff --git a/arch/um/drivers/Kconfig b/arch/um/drivers/Kconfig index fea5a0d522dc..388096fb45a2 100644 --- a/arch/um/drivers/Kconfig +++ b/arch/um/drivers/Kconfig @@ -337,7 +337,7 @@ config UML_NET_SLIRP endmenu config VIRTIO_UML - tristate "UML driver for virtio devices" + bool "UML driver for virtio devices" select VIRTIO help This driver provides support for virtio based paravirtual device diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c index 76b97c2de9a8..023ced2250ea 100644 --- a/arch/um/drivers/virtio_uml.c +++ b/arch/um/drivers/virtio_uml.c @@ -4,12 +4,12 @@ * * Copyright(c) 2019 Intel Corporation * - * This module allows virtio devices to be used over a vhost-user socket. + * This driver allows virtio devices to be used over a vhost-user socket. * * Guest devices can be instantiated by kernel module or command line * parameters. One device will be created for each parameter. Syntax: * - * [virtio_uml.]device=:[:] + * virtio_uml.device=:[:] * where: * := vhost-user socket path to connect * := virtio device id (as in virtio_ids.h) -- cgit v1.2.3