Re: [RFC V2 PATCH 8/12] UIO/Hyper-V: Not load UIO HV driver in the isolation VM.
On Wed, Apr 14, 2021 at 11:20:19PM +0800, Tianyu Lan wrote: > Hi Greg: > Thanks for your review. > > On 4/14/2021 12:00 AM, Greg KH wrote: > > On Tue, Apr 13, 2021 at 11:22:13AM -0400, Tianyu Lan wrote: > > > From: Tianyu Lan > > > > > > UIO HV driver should not load in the isolation VM for security reason. > > > > Why? I need a lot more excuse than that. > > The reason is that ring buffers have been marked as visible to host. > UIO driver will expose these buffers to user space and user space > driver hasn't done some secure check for data from host. This > is considered as insecure in isolation VM. But as this is a VM choice, why did the VM mark those as visible in the first place? thanks, greg k-h
Re: [RFC V2 PATCH 8/12] UIO/Hyper-V: Not load UIO HV driver in the isolation VM.
Hi Greg: Thanks for your review. On 4/14/2021 12:00 AM, Greg KH wrote: On Tue, Apr 13, 2021 at 11:22:13AM -0400, Tianyu Lan wrote: From: Tianyu Lan UIO HV driver should not load in the isolation VM for security reason. Why? I need a lot more excuse than that. The reason is that ring buffers have been marked as visible to host. UIO driver will expose these buffers to user space and user space driver hasn't done some secure check for data from host. This is considered as insecure in isolation VM. Why would the vm allow UIO devices to bind to it if it was not possible? Shouldn't the VM be handling this type of logic and not forcing all individual hyperv drivers to do this? This feels wrong... Hypervisor exposes network and storage devices but can't prohibit guest from binding these devices to UIO driver. You are right. This should not happen in the individual driver and will try handling this in the vmbus driver level. thanks, greg k-h
Re: [RFC V2 PATCH 8/12] UIO/Hyper-V: Not load UIO HV driver in the isolation VM.
On Tue, Apr 13, 2021 at 11:22:13AM -0400, Tianyu Lan wrote: > From: Tianyu Lan > > UIO HV driver should not load in the isolation VM for security reason. Why? I need a lot more excuse than that. Why would the vm allow UIO devices to bind to it if it was not possible? Shouldn't the VM be handling this type of logic and not forcing all individual hyperv drivers to do this? This feels wrong... thanks, greg k-h
Re: [RFC V2 PATCH 8/12] UIO/Hyper-V: Not load UIO HV driver in the isolation VM.
On Tue, Apr 13, 2021 at 11:22:13AM -0400, Tianyu Lan wrote:
> From: Tianyu Lan
>
> UIO HV driver should not load in the isolation VM for security reason.
> Return ENOTSUPP in the hv_uio_probe() in the isolation VM.
>
> Signed-off-by: Tianyu Lan
> ---
> drivers/uio/uio_hv_generic.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index 0330ba99730e..678b021d66f8 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
No driver should be having to add "asm" include files.
>
> #include "../hv/hyperv_vmbus.h"
>
> @@ -241,6 +242,10 @@ hv_uio_probe(struct hv_device *dev,
> void *ring_buffer;
> int ret;
>
> + /* UIO driver should not be loaded in the isolation VM.*/
> + if (hv_is_isolation_supported())
> + return -ENOTSUPP;
> +
> /* Communicating with host has to be via shared memory not hypercall */
> if (!channel->offermsg.monitor_allocated) {
> dev_err(&dev->device, "vmbus channel requires hypercall\n");
> --
> 2.25.1
>
Always run checkpatch.pl on your patches so you do not get grumpy
maintainers telling you to run checkpatch.pl on your patch :(
{sigh}
[RFC V2 PATCH 8/12] UIO/Hyper-V: Not load UIO HV driver in the isolation VM.
From: Tianyu Lan
UIO HV driver should not load in the isolation VM for security reason.
Return ENOTSUPP in the hv_uio_probe() in the isolation VM.
Signed-off-by: Tianyu Lan
---
drivers/uio/uio_hv_generic.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index 0330ba99730e..678b021d66f8 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include "../hv/hyperv_vmbus.h"
@@ -241,6 +242,10 @@ hv_uio_probe(struct hv_device *dev,
void *ring_buffer;
int ret;
+ /* UIO driver should not be loaded in the isolation VM.*/
+ if (hv_is_isolation_supported())
+ return -ENOTSUPP;
+
/* Communicating with host has to be via shared memory not hypercall */
if (!channel->offermsg.monitor_allocated) {
dev_err(&dev->device, "vmbus channel requires hypercall\n");
--
2.25.1
