| Message ID | 20210414144945.3460554-9-ltykernel@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86/Hyper-V: Add Hyper-V Isolation VM support | expand |
On Wed, Apr 14, 2021 at 10:49:41AM -0400, Tianyu Lan wrote: > From: Tianyu Lan <Tianyu.Lan@microsoft.com> > > UIO HV driver should not load in the isolation VM for security reason. > Return ENOTSUPP in the hv_uio_probe() in the isolation VM. What is the "security reason"? After all a user can simply patch the code and just load it anyway..
Hi Stephen: Thanks for your review. On 4/15/2021 12:17 AM, Stephen Hemminger wrote: > On Wed, 14 Apr 2021 17:45:51 +0200 > Greg KH <gregkh@linuxfoundation.org> wrote: > >> On Wed, Apr 14, 2021 at 10:49:41AM -0400, Tianyu Lan wrote: >>> From: Tianyu Lan <Tianyu.Lan@microsoft.com> >>> >>> 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 <Tianyu.Lan@microsoft.com> > > This is debatable, in isolation VM's shouldn't userspace take responsibility > to validate host communication. If that is an issue please participate with > the DPDK community (main user of this) to make sure netvsc userspace driver > has the required checks. > Agree. Will report back to secure team and apply request to add change in userspace netvsc driver. Thanks for advise.
On 4/14/2021 11:45 PM, Greg KH wrote: > On Wed, Apr 14, 2021 at 10:49:41AM -0400, Tianyu Lan wrote: >> From: Tianyu Lan <Tianyu.Lan@microsoft.com> >> >> 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 <Tianyu.Lan@microsoft.com> >> --- >> 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 <linux/hyperv.h> >> #include <linux/vmalloc.h> >> #include <linux/slab.h> >> +#include <asm/mshyperv.h> >> >> #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 >> > > Again you send out known-wrong patches? > > :( > Sorry for noise. Will fix this next version and I think we should make sure user space driver to check data from host. This patch will be removed.
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 <linux/hyperv.h> #include <linux/vmalloc.h> #include <linux/slab.h> +#include <asm/mshyperv.h> #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");