Re: [PATCH v5 5/8] libvduse: Add VDUSE (vDPA Device in Userspace) library

2022-05-19 Thread Yongji Xie 
On Wed, May 18, 2022 at 9:46 PM Stefan Hajnoczi  wrote:
>
> On Wed, May 04, 2022 at 03:40:48PM +0800, Xie Yongji wrote:
> > +static int vduse_queue_update_vring(VduseVirtq *vq, uint64_t desc_addr,
> > +uint64_t avail_addr, uint64_t 
> > used_addr)
> > +{
> > +struct VduseDev *dev = vq->dev;
> > +uint64_t len;
> > +
> > +len = sizeof(struct vring_desc);
> > +vq->vring.desc = iova_to_va(dev, , desc_addr);
> > +assert(len == sizeof(struct vring_desc));
> > +
> > +len = sizeof(struct vring_avail);
> > +vq->vring.avail = iova_to_va(dev, , avail_addr);
> > +assert(len == sizeof(struct vring_avail));
> > +
> > +len = sizeof(struct vring_used);
> > +vq->vring.used = iova_to_va(dev, , used_addr);
> > +assert(len == sizeof(struct vring_used));
>
> Can these assertions be triggered by a malicious virtio driver? For
> example, if a guest is accessing this device, will the vDPA/VDUSE kernel
> code forward the address to QEMU without validation?
>
> If yes, then it's necessary to return an error here instead of aborting.
> A qemu-storage-daemon process might contain multiple VDUSE exports and
> an error in one export shouldn't kill the entire process.
>
> I haven't audited the code, but this is a general issue: if vDPA/VDUSE
> kernel code forwards untrusted inputs to us then we cannot abort or
> crash. Usually the kernel is trusted by userspace but maybe not in this
> case since it may just forward inputs from a malicious VIRTIO driver?

Make sense. And it can also be triggered by the malicious VM in
vhost-vdpa cases. Will fix it in v6.

Thanks,
Yongji

Re: [PATCH v5 5/8] libvduse: Add VDUSE (vDPA Device in Userspace) library

2022-05-18 Thread Stefan Hajnoczi 
On Wed, May 04, 2022 at 03:40:48PM +0800, Xie Yongji wrote:
> +static int vduse_queue_update_vring(VduseVirtq *vq, uint64_t desc_addr,
> +uint64_t avail_addr, uint64_t used_addr)
> +{
> +struct VduseDev *dev = vq->dev;
> +uint64_t len;
> +
> +len = sizeof(struct vring_desc);
> +vq->vring.desc = iova_to_va(dev, , desc_addr);
> +assert(len == sizeof(struct vring_desc));
> +
> +len = sizeof(struct vring_avail);
> +vq->vring.avail = iova_to_va(dev, , avail_addr);
> +assert(len == sizeof(struct vring_avail));
> +
> +len = sizeof(struct vring_used);
> +vq->vring.used = iova_to_va(dev, , used_addr);
> +assert(len == sizeof(struct vring_used));

Can these assertions be triggered by a malicious virtio driver? For
example, if a guest is accessing this device, will the vDPA/VDUSE kernel
code forward the address to QEMU without validation?

If yes, then it's necessary to return an error here instead of aborting.
A qemu-storage-daemon process might contain multiple VDUSE exports and
an error in one export shouldn't kill the entire process.

I haven't audited the code, but this is a general issue: if vDPA/VDUSE
kernel code forwards untrusted inputs to us then we cannot abort or
crash. Usually the kernel is trusted by userspace but maybe not in this
case since it may just forward inputs from a malicious VIRTIO driver?


signature.asc
Description: PGP signature

[PATCH v5 5/8] libvduse: Add VDUSE (vDPA Device in Userspace) library

2022-05-04 Thread Xie Yongji 
VDUSE [1] is a linux framework that makes it possible to implement
software-emulated vDPA devices in userspace. This adds a library
as a subproject to help implementing VDUSE backends in QEMU.

[1] https://www.kernel.org/doc/html/latest/userspace-api/vduse.html

Signed-off-by: Xie Yongji 
---
 MAINTAINERS |5 +
 meson.build |   15 +
 meson_options.txt   |2 +
 scripts/meson-buildoptions.sh   |3 +
 subprojects/libvduse/include/atomic.h   |1 +
 subprojects/libvduse/include/compiler.h |1 +
 subprojects/libvduse/libvduse.c | 1161 +++
 subprojects/libvduse/libvduse.h |  235 
 subprojects/libvduse/linux-headers/linux|1 +
 subprojects/libvduse/meson.build|   10 +
 subprojects/libvduse/standard-headers/linux |1 +
 11 files changed, 1435 insertions(+)
 create mode 12 subprojects/libvduse/include/atomic.h
 create mode 12 subprojects/libvduse/include/compiler.h
 create mode 100644 subprojects/libvduse/libvduse.c
 create mode 100644 subprojects/libvduse/libvduse.h
 create mode 12 subprojects/libvduse/linux-headers/linux
 create mode 100644 subprojects/libvduse/meson.build
 create mode 12 subprojects/libvduse/standard-headers/linux

diff --git a/MAINTAINERS b/MAINTAINERS
index 4113b6fc5c..6de3cbaa1e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3550,6 +3550,11 @@ L: qemu-bl...@nongnu.org
 S: Supported
 F: block/export/fuse.c
 
+VDUSE library
+M: Xie Yongji 
+S: Maintained
+F: subprojects/libvduse/
+
 Replication
 M: Wen Congyang 
 M: Xie Changlong 
diff --git a/meson.build b/meson.build
index 1fe7d257ff..4a0f1a2016 100644
--- a/meson.build
+++ b/meson.build
@@ -1392,6 +1392,21 @@ if get_option('fuse_lseek').allowed()
   endif
 endif
 
+have_libvduse = (targetos == 'linux')
+if get_option('libvduse').enabled()
+if targetos != 'linux'
+error('libvduse requires linux')
+endif
+elif get_option('libvduse').disabled()
+have_libvduse = false
+endif
+
+libvduse = not_found
+if have_libvduse
+  libvduse_proj = subproject('libvduse')
+  libvduse = libvduse_proj.get_variable('libvduse_dep')
+endif
+
 # libbpf
 libbpf = dependency('libbpf', required: get_option('bpf'), method: 
'pkg-config')
 if libbpf.found() and not cc.links('''
diff --git a/meson_options.txt b/meson_options.txt
index af432a4ee6..47955f972d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -231,6 +231,8 @@ option('virtfs', type: 'feature', value: 'auto',
description: 'virtio-9p support')
 option('virtiofsd', type: 'feature', value: 'auto',
description: 'build virtiofs daemon (virtiofsd)')
+option('libvduse', type: 'feature', value: 'auto',
+   description: 'build VDUSE Library')
 
 option('capstone', type: 'combo', value: 'auto',
choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 21366b2102..f725636ea8 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -81,6 +81,7 @@ meson_options_help() {
   printf "%s\n" '  libssh  ssh block device support'
   printf "%s\n" '  libudev Use libudev to enumerate host devices'
   printf "%s\n" '  libusb  libusb support for USB passthrough'
+  printf "%s\n" '  libvdusebuild VDUSE Library'
   printf "%s\n" '  linux-aio   Linux AIO support'
   printf "%s\n" '  linux-io-uring  Linux io_uring support'
   printf "%s\n" '  live-block-migration'
@@ -255,6 +256,8 @@ _meson_option_parse() {
 --disable-libudev) printf "%s" -Dlibudev=disabled ;;
 --enable-libusb) printf "%s" -Dlibusb=enabled ;;
 --disable-libusb) printf "%s" -Dlibusb=disabled ;;
+--enable-libvduse) printf "%s" -Dlibvduse=enabled ;;
+--disable-libvduse) printf "%s" -Dlibvduse=disabled ;;
 --enable-linux-aio) printf "%s" -Dlinux_aio=enabled ;;
 --disable-linux-aio) printf "%s" -Dlinux_aio=disabled ;;
 --enable-linux-io-uring) printf "%s" -Dlinux_io_uring=enabled ;;
diff --git a/subprojects/libvduse/include/atomic.h 
b/subprojects/libvduse/include/atomic.h
new file mode 12
index 00..8c2be64f7b
--- /dev/null
+++ b/subprojects/libvduse/include/atomic.h
@@ -0,0 +1 @@
+../../../include/qemu/atomic.h
\ No newline at end of file
diff --git a/subprojects/libvduse/include/compiler.h 
b/subprojects/libvduse/include/compiler.h
new file mode 12
index 00..de7b70697c
--- /dev/null
+++ b/subprojects/libvduse/include/compiler.h
@@ -0,0 +1 @@
+../../../include/qemu/compiler.h
\ No newline at end of file
diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
new file mode 100644
index 00..ecee9c0568
--- /dev/null
+++ b/subprojects/libvduse/libvduse.c
@@ -0,0 +1,1161 @@
+/*
+ * VDUSE (vDPA Device in Userspace) library
+ *
+ * Copyright (C) 2022 Bytedance Inc. and/or its affiliates. All