Re: Security hole in vhost-vdpa?

Re: Security hole in vhost-vdpa?

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

Re: Security hole in vhost-vdpa?

[Jason Wang]

在 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


> 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

Re: Security hole in vhost-vdpa?

[Michael S. Tsirkin]

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

Re: Security hole in vhost-vdpa?

[Jason Wang]

在 2021/6/10 下午12:30, Michael S. Tsirkin 写道:
> 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 ...


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


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

Re: Security hole in vhost-vdpa?

[Jason Wang]

在 2021/6/10 下午1:00, Gautam Dawar 写道:
> Pls see inline
>
> Regards,
> Gautam
>
> -----Original Message-----
> From: Michael S. Tsirkin<mst@redhat.com>  
> Sent: Thursday, June 10, 2021 10:00 AM
> To: Jason Wang<jasowang@redhat.com>
> Cc: Gautam Dawar<gdawar@xilinx.com>; Harpreet Singh Anand<hanand@xilinx.com>; Martin Petrus Hubertus Habets<martinh@xilinx.com>;virtualization@lists.linux-foundation.org
> Subject: Re: Security hole in vhost-vdpa?
>
> 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 ...
>
> [GD>>] I am working on a solution that is limited to kernel space only and doesn't depend on userspace application (which could be a malicious one).
> Will share more updates in a couple of days.


Cool. Let's see.

Thanks


>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization