| Message ID | 20210414144945.3460554-9-ltykernel@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | x86/Hyper-V: Add Hyper-V Isolation VM support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
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..
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? :(
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.
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");