On Mon, Jun 07, 2021 at 10:10:03AM +0800, Jason Wang wrote:
>
> 在 2021/6/7 上午5:38, Michael S. Tsirkin 写道:
> > On Sun, Jun 06, 2021 at 02:39:48PM +0000, Gautam Dawar wrote:
> > > Hi All,
> > >
> > >
> > > This is in continuation to my findings noted in Bug 213179 and
> > > discussions we
> > > have had in the last couple of weeks over emails.
> > >
> > >
> > > Today, I published the first patch for this issue which adds timeout
> > > based wait
> > > for completion event and also logs a warning message to alert the user/
> > > administrator of the problem.
> > Can't close just finish without waiting for userspace?
>
>
> It works as long as we don't use mmap(). When we map kicks, it looks to me
> there's no way to "revoke" the mapping from userspace?
>
> Thanks
Can't we track these mappings and map some other page there?
Likely no more than one is needed ...
>
> > Then notify userspace about any buffers that did not complete ...
> >
> >
> > > As a next step, the idea is to implement a mechanism to allow vhost-vdpa
> > > module
> > > notify userspace app (QEMU) to close the fd corresponding to the
> > > vhost-vdpa
> > > character device when it is waiting for the completion event in
> > > vhost_vdpa_remove(). Jason confirmed this by saying that we need a new
> > > eventfd/
> > > ioctl to receive hot remove request from kernel.
> > >
> > >
> > > Although, we can proceed to implement changes for the part described
> > > above but
> > > I feel that that the problem is much deeper than that. This mechanism
> > > will just
> > > request the userspace to close the fd and let vhost-vdpa proceed with the
> > > clean-up. However, IMHO things should be under more control of kernel
> > > space
> > > than the user space.
> > >
> > >
> > > The problem I am trying to highlight is that a malicious user-space
> > > application
> > > can render any module registering a vDPA device to hang in their
> > > de-initialization sequence. This will typically surface when
> > > vdpa_device_unregister() is called from the function responsible for
> > > module
> > > unload leading rmmod commands to not return, forever.
> > >
> > >
> > > To prove my point, I created a simple C program (test_vdpa.c) that opens
> > > the
> > > vhost-vdpa character device and never exits. The logs (test_logs.txt)
> > > show that
> > > after registering the vDPA device from sfc driver, vhost-vdpa module
> > > creates
> > > the char device /dev/vhost-vdpa-0 for it. As this is available to all
> > > apps in
> > > the userspace, the malicious app (./block_vdpa_unload) opens this device
> > > and
> > > goes to infinite sleep. At this time, when module unload (rmmod sfc) is
> > > called,
> > > it hangs and the following print informs the user/admin of this state with
> > > following message:
> > >
> > > [ 8180.053647] vhost-vdpa-0: vhost_vdpa_remove waiting for
> > > /dev/vhost-vdpa-0
> > > to be closed
> > >
> > >
> > > Finally, when block_vdpa_unload is killed, vhost_vdpa_remove() unblocks
> > > and sfc
> > > module is unloaded.
> > >
> > >
> > > With such application running in userspace, a kernel module (that
> > > registered
> > > corresponding vDPA device) will hang during unload sequence. Such control
> > > of
> > > the userspace application on the system resources should certainly be
> > > prevented.
> > >
> > > To me, this seems to be a serious issue and requires modifications in the
> > > way
> > > it is currently handled in vhost-vdpa (and other modules (VFIO?) with
> > > similar
> > > implementation).
> > >
> > > Let me know what you think.
> > >
> > >
> > > Regards,
> > >
> > > Gautam Dawar
> > >
> > > #include <sys/stat.h>
> > > #include <unistd.h>
> > > #include <stdlib.h>
> > > #include <stdio.h>
> > > #include <fcntl.h>
> > > #include <errno.h>
> > >
> > > int main(int argc, char **argv)
> > > {
> > > unsigned int index;
> > > char dev_path[30];
> > > int fd;
> > >
> > > if (argc != 2) {
> > > printf("Usage: %s <vhost-vdpa device index>\n", argv[0]);
> > > return -1;
> > > }
> > >
> > > index = strtoul(argv[1], NULL, 10);
> > >
> > > snprintf(dev_path, sizeof(dev_path), "/dev/vhost-vdpa-%u", index);
> > > fd = open(dev_path, O_RDWR);
> > > if(fd < 0)
> > > {
> > > printf("Failed to open %s, errno: %d!\n", dev_path, errno);
> > > return 1;
> > > }
> > >
> > > printf("Blocking unload of driver that registered vDPA device"
> > > " corresponding to cdev %s created by vhost-vdpa\n", dev_path);
> > > while (1)
> > > sleep(1);
> > >
> > > close(fd);
> > > return 0;
> > > }
> > > [root@ndr730p ~]# ~/create_vdpa_device.sh
> > >
> > > [root@ndr730p ~]# ll /dev/vhost-vdpa-0
> > > crw------- 1 root root 240, 0 Jun 6 19:59 /dev/vhost-vdpa-0
> > >
> > > [root@ndr730p ~]# ./block_vdpa_unload 0 &
> > > [1] 10930
> > > Blocking unload of driver that registered vDPA device corresponding to
> > > cdev /dev/vhost-vdpa-0 created by vhost-vdpa
> > >
> > > [root@ndr730p ~]# rmmod sfc
> > > [ 8179.010520] sfc_ef100 0000:06:00.4: ef100_vdpa_delete: Calling vdpa
> > > unregister device
> > > [ 8180.053647] vhost-vdpa-0: vhost_vdpa_remove waiting for
> > > /dev/vhost-vdpa-0 to be closed
> > >
> > > [root@ndr730p ~]# kill -9 10930
> > > [ 8218.392897] sfc_ef100 0000:06:00.0: shutdown successful
> > >
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
