[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 4/7] vhost-pci-slave: add vhost-pci slave imp
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v3 4/7] vhost-pci-slave: add vhost-pci slave implementation |
Date: |
Tue, 5 Dec 2017 15:56:01 +0000 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Tue, Dec 05, 2017 at 11:33:13AM +0800, Wei Wang wrote:
> +static int vp_slave_write(CharBackend *chr_be, VhostUserMsg *msg)
> +{
> + int size;
> +
> + if (!msg) {
> + return 0;
> + }
> +
> + /* The payload size has been assigned, plus the header size here */
> + size = msg->size + VHOST_USER_HDR_SIZE;
> + msg->flags &= ~VHOST_USER_VERSION_MASK;
> + msg->flags |= VHOST_USER_VERSION;
> +
> + return qemu_chr_fe_write_all(chr_be, (const uint8_t *)msg, size)
> + == size ? 0 : -1;
> +}
qemu_chr_fe_write_all() is a blocking operation. If the socket fd
cannot accept more data then this thread will block!
This is a reliability problem. If the vhost-user master process hangs
then the vhost-pci vhost-user slave will also hang :(.
Please implement vhost-pci so that it does not hang. A guest with
multiple vhost-pci devices should work even if one or more of them
cannot communicate with the vhost-pci master. This is necessary for
preventing denial-of-service on a software-defined network switch, for
example.
> +static void vp_slave_set_vring_num(VhostPCINet *vpnet, VhostUserMsg *msg)
> +{
> + struct vhost_vring_state *state = &msg->payload.state;
> +
> + vpnet->metadata->vq[state->index].vring_num = state->num;
The vhost-pci code cannot trust vhost-user input. This function allows
the vhost-user master to perform out-of-bounds memory stores by setting
state->index outside the vq[] array.
All input must be validated! The security model is:
1. Guest must be able to corrupt QEMU memory or execute arbitrary code.
2. vhost-user master guest must not be able to corrupt vhost-user slave
guest memory or execute arbitrary code.
3. vhost-user master must not be able to corrupt vhost-user memory or
execute arbitrary code in vhost-user slave.
4. vhost-user slave must not be able to corrupt vhost-user memory or
execute arbitrary code in vhost-user master.
The only thing that is allowed is vhost-user slave QEMU and guest may
write to vhost-user master guest memory.
> +void vp_slave_read(void *opaque, const uint8_t *buf, int size)
> +{
> + int ret, fd_num, fds[VHOST_MEMORY_MAX_NREGIONS];
> + VhostUserMsg msg;
> + uint8_t *p = (uint8_t *) &msg;
> + VhostPCINet *vpnet = (VhostPCINet *)opaque;
> + CharBackend *chr_be = &vpnet->chr_be;
> +
> + if (size != VHOST_USER_HDR_SIZE) {
> + error_report("%s: wrong message size received %d", __func__, size);
> + return;
> + }
> +
> + memcpy(p, buf, VHOST_USER_HDR_SIZE);
> +
> + if (msg.size) {
> + p += VHOST_USER_HDR_SIZE;
> + size = qemu_chr_fe_read_all(chr_be, p, msg.size);
This is a blocking operation. See my comment about
qemu_chr_fe_write_all().
This is also a buffer overflow since msg.size is not validated. All
input from the vhost-user master needs to be validated.
signature.asc
Description: PGP signature
- Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net, (continued)
- Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net, Michael S. Tsirkin, 2017/12/05
- Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net, Michael S. Tsirkin, 2017/12/05
- Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net, Stefan Hajnoczi, 2017/12/05
- Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net, Michael S. Tsirkin, 2017/12/05
- Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net, Cornelia Huck, 2017/12/05
- Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net, Michael S. Tsirkin, 2017/12/05
- Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net, Wei Wang, 2017/12/06
- Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net, Stefan Hajnoczi, 2017/12/06
[Qemu-devel] [PATCH v3 3/7] virtio/virtio-pci.c: add vhost-pci-net-pci, Wei Wang, 2017/12/04
[Qemu-devel] [PATCH v3 4/7] vhost-pci-slave: add vhost-pci slave implementation, Wei Wang, 2017/12/04
[Qemu-devel] [PATCH v3 5/7] vhost-user: VHOST_USER_SET_VHOST_PCI msg, Wei Wang, 2017/12/04
[Qemu-devel] [PATCH v3 6/7] vhost-pci-slave: handle VHOST_USER_SET_VHOST_PCI, Wei Wang, 2017/12/04
[Qemu-devel] [PATCH v3 7/7] virtio/vhost.c: vhost-pci needs remote gpa, Wei Wang, 2017/12/04