From: Stefan Hajnoczi <stefanha@redhat.com>
To: Elena Ufimtseva <elena.ufimtseva@oracle.com>
Cc: john.g.johnson@oracle.com, jag.raman@oracle.com,
swapnil.ingle@nutanix.com, john.levon@nutanix.com,
qemu-devel@nongnu.org, alex.williamson@redhat.com,
thanos.makatos@nutanix.com
Subject: Re: [PATCH RFC v2 06/16] vfio-user: negotiate version with remote server
Date: Tue, 24 Aug 2021 16:59:17 +0100 [thread overview]
Message-ID: <YSUXVQJwkubjpp9x@stefanha-x1.localdomain> (raw)
In-Reply-To: <a14c2c09f41ddda83cd710516cac8d210ec9db08.1629131628.git.elena.ufimtseva@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 4808 bytes --]
On Mon, Aug 16, 2021 at 09:42:39AM -0700, Elena Ufimtseva wrote:
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 7005d9f891..eae33e746f 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3397,6 +3397,12 @@ static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
> proxy->flags |= VFIO_PROXY_SECURE;
> }
>
> + vfio_user_validate_version(vbasedev, &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + goto error;
> + }
> +
> vbasedev->name = g_strdup_printf("VFIO user <%s>", udev->sock_name);
> vbasedev->dev = DEVICE(vdev);
> vbasedev->fd = -1;
> @@ -3404,6 +3410,9 @@ static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
> vbasedev->no_mmap = false;
> vbasedev->ops = &vfio_user_pci_ops;
>
> +error:
Missing return before error label? We shouldn't disconnect in the
success case.
> + vfio_user_disconnect(proxy);
> + error_prepend(errp, VFIO_MSG_PREFIX, vdev->vbasedev.name);
> }
>
> static void vfio_user_instance_finalize(Object *obj)
> diff --git a/hw/vfio/user.c b/hw/vfio/user.c
> index 2fcc77d997..e89464a571 100644
> --- a/hw/vfio/user.c
> +++ b/hw/vfio/user.c
> @@ -23,9 +23,16 @@
> #include "io/channel-socket.h"
> #include "io/channel-util.h"
> #include "sysemu/iothread.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qnull.h"
> +#include "qapi/qmp/qstring.h"
> +#include "qapi/qmp/qnum.h"
> #include "user.h"
>
> static uint64_t max_xfer_size = VFIO_USER_DEF_MAX_XFER;
> +static uint64_t max_send_fds = VFIO_USER_DEF_MAX_FDS;
> +static int wait_time = 1000; /* wait 1 sec for replies */
> static IOThread *vfio_user_iothread;
>
> static void vfio_user_shutdown(VFIOProxy *proxy);
> @@ -34,7 +41,14 @@ static void vfio_user_send_locked(VFIOProxy *proxy, VFIOUserHdr *msg,
> VFIOUserFDs *fds);
> static void vfio_user_send(VFIOProxy *proxy, VFIOUserHdr *msg,
> VFIOUserFDs *fds);
> +static void vfio_user_request_msg(VFIOUserHdr *hdr, uint16_t cmd,
> + uint32_t size, uint32_t flags);
> +static void vfio_user_send_recv(VFIOProxy *proxy, VFIOUserHdr *msg,
> + VFIOUserFDs *fds, int rsize, int flags);
>
> +/* vfio_user_send_recv flags */
> +#define NOWAIT 0x1 /* do not wait for reply */
> +#define NOIOLOCK 0x2 /* do not drop iolock */
Please use "BQL", it's a widely used term while "iolock" isn't used:
s/IOLOCK/BQL/
>
> /*
> * Functions called by main, CPU, or iothread threads
> @@ -333,6 +347,79 @@ static void vfio_user_cb(void *opaque)
> * Functions called by main or CPU threads
> */
>
> +static void vfio_user_send_recv(VFIOProxy *proxy, VFIOUserHdr *msg,
> + VFIOUserFDs *fds, int rsize, int flags)
> +{
> + VFIOUserReply *reply;
> + bool iolock = 0;
> +
> + if (msg->flags & VFIO_USER_NO_REPLY) {
> + error_printf("vfio_user_send_recv on async message\n");
> + return;
> + }
> +
> + /*
> + * We may block later, so use a per-proxy lock and let
> + * the iothreads run while we sleep unless told no to
s/no/not/
> +int vfio_user_validate_version(VFIODevice *vbasedev, Error **errp)
> +{
> + g_autofree VFIOUserVersion *msgp;
> + GString *caps;
> + int size, caplen;
> +
> + caps = caps_json();
> + caplen = caps->len + 1;
> + size = sizeof(*msgp) + caplen;
> + msgp = g_malloc0(size);
> +
> + vfio_user_request_msg(&msgp->hdr, VFIO_USER_VERSION, size, 0);
> + msgp->major = VFIO_USER_MAJOR_VER;
> + msgp->minor = VFIO_USER_MINOR_VER;
> + memcpy(&msgp->capabilities, caps->str, caplen);
> + g_string_free(caps, true);
> +
> + vfio_user_send_recv(vbasedev->proxy, &msgp->hdr, NULL, 0, 0);
> + if (msgp->hdr.flags & VFIO_USER_ERROR) {
> + error_setg_errno(errp, msgp->hdr.error_reply, "version reply");
> + return -1;
> + }
> +
> + if (msgp->major != VFIO_USER_MAJOR_VER ||
> + msgp->minor > VFIO_USER_MINOR_VER) {
> + error_setg(errp, "incompatible server version");
> + return -1;
> + }
> + if (caps_check(msgp->minor, (char *)msgp + sizeof(*msgp), errp) != 0) {
The reply is untrusted so we cannot treat it as a NUL-terminated string
yet. The final byte msgp->capabilities[] needs to be checked first.
Please be careful about input validation, I might miss something so it's
best if you audit the patches too. QEMU must not trust the device
emulation process and vice versa.
> + return -1;
> + }
> +
> + return 0;
> +}
> --
> 2.25.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2021-08-24 16:12 UTC|newest]
Thread overview: 108+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-16 16:42 [PATCH RFC v2 00/16] vfio-user implementation Elena Ufimtseva
2021-08-16 16:42 ` [PATCH RFC v2 01/16] vfio-user: introduce vfio-user protocol specification Elena Ufimtseva
2021-08-17 23:04 ` Alex Williamson
2021-08-19 9:28 ` Swapnil Ingle
2021-08-19 15:32 ` John Johnson
2021-08-19 16:26 ` Alex Williamson
2021-08-16 16:42 ` [PATCH RFC v2 02/16] vfio-user: add VFIO base abstract class Elena Ufimtseva
2021-08-16 16:42 ` [PATCH RFC v2 03/16] vfio-user: Define type vfio_user_pci_dev_info Elena Ufimtseva
2021-08-24 13:52 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 04/16] vfio-user: connect vfio proxy to remote server Elena Ufimtseva
2021-08-18 18:47 ` Alex Williamson
2021-08-19 14:10 ` John Johnson
2021-08-24 14:15 ` Stefan Hajnoczi
2021-08-30 3:00 ` John Johnson
2021-09-07 13:21 ` Stefan Hajnoczi
2021-09-09 5:11 ` John Johnson
2021-09-09 6:29 ` Stefan Hajnoczi
2021-09-10 5:25 ` John Johnson
2021-09-13 12:35 ` Stefan Hajnoczi
2021-09-13 17:23 ` John Johnson
2021-09-14 13:06 ` Stefan Hajnoczi
2021-09-15 0:21 ` John Johnson
2021-09-15 13:04 ` Stefan Hajnoczi
2021-09-15 19:14 ` John Johnson
2021-09-16 11:49 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 05/16] vfio-user: define VFIO Proxy and communication functions Elena Ufimtseva
2021-08-24 15:14 ` Stefan Hajnoczi
2021-08-30 3:04 ` John Johnson
2021-09-07 13:35 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 06/16] vfio-user: negotiate version with remote server Elena Ufimtseva
2021-08-24 15:59 ` Stefan Hajnoczi [this message]
2021-08-30 3:08 ` John Johnson
2021-09-07 13:52 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 07/16] vfio-user: get device info Elena Ufimtseva
2021-08-24 16:04 ` Stefan Hajnoczi
2021-08-30 3:11 ` John Johnson
2021-09-07 13:54 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 08/16] vfio-user: get region info Elena Ufimtseva
2021-09-07 14:31 ` Stefan Hajnoczi
2021-09-09 5:35 ` John Johnson
2021-09-09 5:59 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 09/16] vfio-user: region read/write Elena Ufimtseva
2021-09-07 14:41 ` Stefan Hajnoczi
2021-09-07 17:24 ` John Levon
2021-09-09 6:00 ` John Johnson
2021-09-09 12:05 ` John Levon
2021-09-10 6:07 ` John Johnson
2021-09-10 12:16 ` John Levon
2021-08-16 16:42 ` [PATCH RFC v2 10/16] vfio-user: pci_user_realize PCI setup Elena Ufimtseva
2021-09-07 15:00 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 11/16] vfio-user: get and set IRQs Elena Ufimtseva
2021-09-07 15:14 ` Stefan Hajnoczi
2021-09-09 5:50 ` John Johnson
2021-09-09 13:50 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 12/16] vfio-user: proxy container connect/disconnect Elena Ufimtseva
2021-09-08 8:30 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 13/16] vfio-user: dma map/unmap operations Elena Ufimtseva
2021-09-08 9:16 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 14/16] vfio-user: dma read/write operations Elena Ufimtseva
2021-09-08 9:51 ` Stefan Hajnoczi
2021-09-08 11:03 ` John Levon
2021-08-16 16:42 ` [PATCH RFC v2 15/16] vfio-user: pci reset Elena Ufimtseva
2021-09-08 9:56 ` Stefan Hajnoczi
2021-08-16 16:42 ` [PATCH RFC v2 16/16] vfio-user: migration support Elena Ufimtseva
2021-09-08 10:04 ` Stefan Hajnoczi
2021-08-27 17:53 ` [PATCH RFC server v2 00/11] vfio-user server in QEMU Jagannathan Raman
2021-08-27 17:53 ` [PATCH RFC server v2 01/11] vfio-user: build library Jagannathan Raman
2021-08-27 18:05 ` Jag Raman
2021-09-08 12:25 ` Stefan Hajnoczi
2021-09-10 15:21 ` Philippe Mathieu-Daudé
2021-09-13 12:15 ` Stefan Hajnoczi
2021-09-10 15:20 ` Philippe Mathieu-Daudé
2021-09-10 17:08 ` Jag Raman
2021-09-11 22:29 ` John Levon
2021-09-13 10:19 ` Philippe Mathieu-Daudé
2021-08-27 17:53 ` [PATCH RFC server v2 02/11] vfio-user: define vfio-user object Jagannathan Raman
2021-09-08 12:37 ` Stefan Hajnoczi
2021-09-10 14:04 ` Jag Raman
2021-08-27 17:53 ` [PATCH RFC server v2 03/11] vfio-user: instantiate vfio-user context Jagannathan Raman
2021-09-08 12:40 ` Stefan Hajnoczi
2021-09-10 14:58 ` Jag Raman
2021-08-27 17:53 ` [PATCH RFC server v2 04/11] vfio-user: find and init PCI device Jagannathan Raman
2021-09-08 12:43 ` Stefan Hajnoczi
2021-09-10 15:02 ` Jag Raman
2021-08-27 17:53 ` [PATCH RFC server v2 05/11] vfio-user: run vfio-user context Jagannathan Raman
2021-09-08 12:58 ` Stefan Hajnoczi
2021-09-08 13:37 ` John Levon
2021-09-08 15:02 ` Stefan Hajnoczi
2021-09-08 15:21 ` John Levon
2021-09-08 15:46 ` Stefan Hajnoczi
2021-08-27 17:53 ` [PATCH RFC server v2 06/11] vfio-user: handle PCI config space accesses Jagannathan Raman
2021-09-09 7:27 ` Stefan Hajnoczi
2021-09-10 16:22 ` Jag Raman
2021-09-13 12:13 ` Stefan Hajnoczi
2021-08-27 17:53 ` [PATCH RFC server v2 07/11] vfio-user: handle DMA mappings Jagannathan Raman
2021-09-09 7:29 ` Stefan Hajnoczi
2021-08-27 17:53 ` [PATCH RFC server v2 08/11] vfio-user: handle PCI BAR accesses Jagannathan Raman
2021-09-09 7:37 ` Stefan Hajnoczi
2021-09-10 16:36 ` Jag Raman
2021-08-27 17:53 ` [PATCH RFC server v2 09/11] vfio-user: handle device interrupts Jagannathan Raman
2021-09-09 7:40 ` Stefan Hajnoczi
2021-08-27 17:53 ` [PATCH RFC server v2 10/11] vfio-user: register handlers to facilitate migration Jagannathan Raman
2021-09-09 8:14 ` Stefan Hajnoczi
2021-08-27 17:53 ` [PATCH RFC server v2 11/11] vfio-user: acceptance test Jagannathan Raman
2021-09-08 10:08 ` [PATCH RFC server v2 00/11] vfio-user server in QEMU Stefan Hajnoczi
2021-09-08 12:06 ` Jag Raman
2021-09-09 8:17 ` Stefan Hajnoczi
2021-09-10 14:02 ` Jag Raman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YSUXVQJwkubjpp9x@stefanha-x1.localdomain \
--to=stefanha@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=elena.ufimtseva@oracle.com \
--cc=jag.raman@oracle.com \
--cc=john.g.johnson@oracle.com \
--cc=john.levon@nutanix.com \
--cc=qemu-devel@nongnu.org \
--cc=swapnil.ingle@nutanix.com \
--cc=thanos.makatos@nutanix.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).