Re: Re: [PATCH v7 00/12] Introduce VDUSE - vDPA Device in Userspace

[Yongji Xie <xieyongji@bytedance.com>]

[-- Attachment #1: Type: text/plain, Size: 8219 bytes --]

On Thu, May 20, 2021 at 2:06 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, May 17, 2021 at 05:55:01PM +0800, Xie Yongji wrote:
> > This series introduces a framework, which can be used to implement
> > vDPA Devices in a userspace program. The work consist of two parts:
> > control path forwarding and data path offloading.
> >
> > In the control path, the VDUSE driver will make use of message
> > mechnism to forward the config operation from vdpa bus driver
> > to userspace. Userspace can use read()/write() to receive/reply
> > those control messages.
> >
> > In the data path, the core is mapping dma buffer into VDUSE
> > daemon's address space, which can be implemented in different ways
> > depending on the vdpa bus to which the vDPA device is attached.
> >
> > In virtio-vdpa case, we implements a MMU-based on-chip IOMMU driver with
> > bounce-buffering mechanism to achieve that. And in vhost-vdpa case, the dma
> > buffer is reside in a userspace memory region which can be shared to the
> > VDUSE userspace processs via transferring the shmfd.
> >
> > The details and our user case is shown below:
> >
> > ------------------------    -------------------------   ----------------------------------------------
> > |            Container |    |              QEMU(VM) |   |                               VDUSE daemon |
> > |       ---------      |    |  -------------------  |   | ------------------------- ---------------- |
> > |       |dev/vdx|      |    |  |/dev/vhost-vdpa-x|  |   | | vDPA device emulation | | block driver | |
> > ------------+-----------     -----------+------------   -------------+----------------------+---------
> >             |                           |                            |                      |
> >             |                           |                            |                      |
> > ------------+---------------------------+----------------------------+----------------------+---------
> > |    | block device |           |  vhost device |            | vduse driver |          | TCP/IP |    |
> > |    -------+--------           --------+--------            -------+--------          -----+----    |
> > |           |                           |                           |                       |        |
> > | ----------+----------       ----------+-----------         -------+-------                |        |
> > | | virtio-blk driver |       |  vhost-vdpa driver |         | vdpa device |                |        |
> > | ----------+----------       ----------+-----------         -------+-------                |        |
> > |           |      virtio bus           |                           |                       |        |
> > |   --------+----+-----------           |                           |                       |        |
> > |                |                      |                           |                       |        |
> > |      ----------+----------            |                           |                       |        |
> > |      | virtio-blk device |            |                           |                       |        |
> > |      ----------+----------            |                           |                       |        |
> > |                |                      |                           |                       |        |
> > |     -----------+-----------           |                           |                       |        |
> > |     |  virtio-vdpa driver |           |                           |                       |        |
> > |     -----------+-----------           |                           |                       |        |
> > |                |                      |                           |    vdpa bus           |        |
> > |     -----------+----------------------+---------------------------+------------           |        |
> > |                                                                                        ---+---     |
> > -----------------------------------------------------------------------------------------| NIC |------
> >                                                                                          ---+---
> >                                                                                             |
> >                                                                                    ---------+---------
> >                                                                                    | Remote Storages |
> >                                                                                    -------------------
> >
> > We make use of it to implement a block device connecting to
> > our distributed storage, which can be used both in containers and
> > VMs. Thus, we can have an unified technology stack in this two cases.
> >
> > To test it with null-blk:
> >
> >   $ qemu-storage-daemon \
> >       --chardev socket,id=charmonitor,path=/tmp/qmp.sock,server,nowait \
> >       --monitor chardev=charmonitor \
> >       --blockdev driver=host_device,cache.direct=on,aio=native,filename=/dev/nullb0,node-name=disk0 \
> >       --export type=vduse-blk,id=test,node-name=disk0,writable=on,name=vduse-null,num-queues=16,queue-size=128
> >
> > The qemu-storage-daemon can be found at https://github.com/bytedance/qemu/tree/vduse
> >
> > To make the userspace VDUSE processes such as qemu-storage-daemon able to
> > run unprivileged. We did some works on virtio driver to avoid trusting
> > device, including:
> >
> >   - validating the device status:
> >
> >     * https://lore.kernel.org/lkml/20210517093428.670-1-xieyongji@bytedance.com/
> >
> >   - validating the used length:
> >
> >     * https://lore.kernel.org/lkml/20210517090836.533-1-xieyongji@bytedance.com/
> >
> >   - validating the device config:
> >
> >     * patch 4 ("virtio-blk: Add validation for block size in config space")
> >
> >   - validating the device response:
> >
> >     * patch 5 ("virtio_scsi: Add validation for residual bytes from response")
> >
> > Since I'm not sure if I missing something during auditing, especially on some
> > virtio device drivers that I'm not familiar with, now we only support emualting
> > a few vDPA devices by default, including: virtio-net device, virtio-blk device,
> > virtio-scsi device and virtio-fs device. This limitation can help to reduce
> > security risks.
>
> I suspect there are a lot of assumptions even with these 4.
> Just what are the security assumptions and guarantees here?

The attack surface from a virtio device is limited with IOMMU enabled.
It should be able to avoid security risk if we can validate all data
such as config space and used length from device in device driver.

> E.g. it seems pretty clear that exposing a malformed FS
> to a random kernel config can cause untold mischief.
>
> Things like virtnet_send_command are also an easy way for
> the device to DOS the kernel. And before you try to add
> an arbitrary timeout there - please don't,
> the fix is moving things that must be guaranteed into kernel
> and making things that are not guaranteed asynchronous.
> Right now there are some things that happen with locks taken,
> where if we don't wait for device we lose the ability to report failures
> to userspace. E.g. all kind of netlink things are like this.
> One can think of a bunch of ways to address this, this
> needs to be discussed with the relevant subsystem maintainers.
>
>
> If I were you I would start with one type of device, and as simple one
> as possible.
>

Make sense to me. The virtio-blk device might be a good start. We
already have some existing interface like NBD to do similar things.

>
>
> > When a sysadmin trusts the userspace process enough, it can relax
> > the limitation with a 'allow_unsafe_device_emulation' module parameter.
>
> That's not a great security interface. It's a global module specific knob
> that just allows any userspace to emulate anything at all.
> Coming up with a reasonable interface isn't going to be easy.
> For now maybe just have people patch their kernels if they want to
> move fast and break things.
>

OK. A reasonable interface can be added if we need it in the future.

Thanks,
Yongji

[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 46427 bytes --]