Re: Security hole in vhost-vdpa?

在 2021/6/10 下午12:30, Michael S. Tsirkin 写道:

On Mon, Jun 07, 2021 at 10:10:03AM +0800, Jason Wang wrote:

quoted

在 2021/6/7 上午5:38, Michael S. Tsirkin 写道:

quoted

On Sun, Jun 06, 2021 at 02:39:48PM +0000, Gautam Dawar wrote:

quoted

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 ...

I think we can but if I understand correctly about how mm guys. Such 
"revoking" is expected to be done only via munmap(). Doing that is 
kernel might be tricky and hard to be correct.

If I was wrong, we can try to do that and post RFC to see if it works.

Thanks

(Note that this "security hole" is not vDPA specific, VFIO and other 
userspace driver subsystem may have the similar "issue").

Thanks

quoted

quoted

Then notify userspace about any buffers that did not complete ...

quoted

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