diff mbox

[net-next,1/3] VMCI: only load on VMware hypervisor

Message ID KL1P15301MB00083C97259FF2A700C78442BF8D0@KL1P15301MB0008.APCP153.PROD.OUTLOOK.COM
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Dexuan Cui Aug. 15, 2017, 10:13 p.m. UTC
Without the patch, vmw_vsock_vmci_transport.ko and vmw_vmci.ko can
automatically load when an application creates an AF_VSOCK socket.

This is the expected good behavior on VMware hypervisor, but as we
are going to add hv_sock.ko (i.e. Hyper-V transport for AF_VSOCK), we
should make sure vmw_vsock_vmci_transport.ko doesn't load on Hyper-V,
otherwise there is a -EBUSY conflict when both vmw_vsock_vmci_transport.ko
and hv_sock.ko try to call vsock_core_init() on Hyper-V.

On the other hand, hv_sock.ko can only load on Hyper-V, because it
depends on hv_vmbus.ko, which detects Hyper-V in hv_acpi_init().

KVM's vsock_virtio_transport doesn't have the issue because it doesn't
define MODULE_ALIAS_NETPROTO(PF_VSOCK).

Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: Alok Kataria <akataria@vmware.com>
Cc: Andy King <acking@vmware.com>
Cc: Adit Ranadive <aditr@vmware.com>
Cc: George Zhang <georgezhang@vmware.com>
Cc: Jorgen Hansen <jhansen@vmware.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/misc/vmw_vmci/vmci_driver.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

David Miller Aug. 16, 2017, 6:06 p.m. UTC | #1
From: Dexuan Cui <decui@microsoft.com>
Date: Tue, 15 Aug 2017 22:13:29 +0000

> +	/*
> +	 * Check if we are running on VMware's hypervisor and bail out
> +	 * if we are not.
> +	 */
> +	if (x86_hyper != &x86_hyper_vmware)
> +		return -ENODEV;

This symbol is only available when CONFIG_HYPERVISOR_GUEST is defined.
But this driver does not have a Kconfig dependency on that symbol so
the build can fail in some configurations.
Jorgen Hansen Aug. 16, 2017, 7:44 p.m. UTC | #2
> On Aug 16, 2017, at 12:13 AM, Dexuan Cui <decui@microsoft.com> wrote:

> 

> 

> Without the patch, vmw_vsock_vmci_transport.ko and vmw_vmci.ko can

> automatically load when an application creates an AF_VSOCK socket.

> 

> This is the expected good behavior on VMware hypervisor, but as we

> are going to add hv_sock.ko (i.e. Hyper-V transport for AF_VSOCK), we

> should make sure vmw_vsock_vmci_transport.ko doesn't load on Hyper-V,

> otherwise there is a -EBUSY conflict when both vmw_vsock_vmci_transport.ko

> and hv_sock.ko try to call vsock_core_init() on Hyper-V.


The VMCI driver (vmw_vmci.ko) is used both by the VMware guest support (VMware Tools primarily) and by our Workstation product. Always disabling the VMCI driver on Hyper-V means that user won’t be able to run Workstation nested in Linux VMs on Hyper-V. Since the VMCI driver itself isn’t the problem here, maybe we could move the check to vmw_vsock_vmci_transport.ko? Ideally, there should be some way for a user to have access to both protocols, but for now disabling the VMCI socket transport for Hyper-V (possibly with a module option to skip that check and always load it) but leaving the VMCI driver functional would be better,

Thanks,
Jorgen
Dexuan Cui Aug. 16, 2017, 10:33 p.m. UTC | #3
> From: Jorgen S. Hansen [mailto:jhansen@vmware.com]

> > Without the patch, vmw_vsock_vmci_transport.ko and vmw_vmci.ko can

> > automatically load when an application creates an AF_VSOCK socket.

> >

> > This is the expected good behavior on VMware hypervisor, but as we

> > are going to add hv_sock.ko (i.e. Hyper-V transport for AF_VSOCK), we

> > should make sure vmw_vsock_vmci_transport.ko doesn't load on Hyper-V,

> > otherwise there is a -EBUSY conflict when both vmw_vsock_vmci_transport.ko

> > and hv_sock.ko try to call vsock_core_init() on Hyper-V.

> 

> The VMCI driver (vmw_vmci.ko) is used both by the VMware guest support

> (VMware Tools primarily) and by our Workstation product. Always disabling the

> VMCI driver on Hyper-V means that user won’t be able to run Workstation

> nested in Linux VMs on Hyper-V. Since the VMCI driver itself isn’t the problem

> here, maybe we could move the check to vmw_vsock_vmci_transport.ko?

> Ideally, there should be some way for a user to have access to both protocols,

> but for now disabling the VMCI socket transport for Hyper-V (possibly with a

> module option to skip that check and always load it) but leaving the VMCI driver

> functional would be better,

> 

> Jorgen


Thank you for explaining the background!
Then I'll make a new patch, following your suggestion.

-- Dexuan
Dexuan Cui Aug. 17, 2017, 8:10 a.m. UTC | #4
> From: Dexuan Cui

> Sent: Wednesday, August 16, 2017 15:34

> > From: Jorgen S. Hansen [mailto:jhansen@vmware.com]

> > > Without the patch, vmw_vsock_vmci_transport.ko and vmw_vmci.ko can

> > > automatically load when an application creates an AF_VSOCK socket.

> > >

> > > This is the expected good behavior on VMware hypervisor, but as we

> > > are going to add hv_sock.ko (i.e. Hyper-V transport for AF_VSOCK), we

> > > should make sure vmw_vsock_vmci_transport.ko doesn't load on Hyper-

> V,

> > > otherwise there is a -EBUSY conflict when both

> vmw_vsock_vmci_transport.ko

> > > and hv_sock.ko try to call vsock_core_init() on Hyper-V.

> >

> > The VMCI driver (vmw_vmci.ko) is used both by the VMware guest support

> > (VMware Tools primarily) and by our Workstation product. Always

> disabling the

> > VMCI driver on Hyper-V means that user won’t be able to run Workstation

> > nested in Linux VMs on Hyper-V. Since the VMCI driver itself isn’t the

> problem

> > here, maybe we could move the check to vmw_vsock_vmci_transport.ko?

> > Ideally, there should be some way for a user to have access to both

> protocols,

> > but for now disabling the VMCI socket transport for Hyper-V (possibly with

> a

> > module option to skip that check and always load it) but leaving the VMCI

> driver

> > functional would be better,

> >

> > Jorgen

> 

> Thank you for explaining the background!

> Then I'll make a new patch, following your suggestion.

> 

> -- Dexuan


Hi Jorgen, David,

Just now I posted a new patch
 "[PATCH] vsock: only load vmci transport on VMware hypervisor by default"
to replace this patch.

@Jorgen: 
FWIW, with the new patch, when I create an AF_VSOCK sockets on Hyper-V,
vmw_vmci.ko is also automatically loaded and 3 lines of kernel messages are
printed, but I think I'm OK with this, since it's harmless.

-- Dexuan
diff mbox

Patch

diff --git a/drivers/misc/vmw_vmci/vmci_driver.c b/drivers/misc/vmw_vmci/vmci_driver.c
index d7eaf1e..1789ea7 100644
--- a/drivers/misc/vmw_vmci/vmci_driver.c
+++ b/drivers/misc/vmw_vmci/vmci_driver.c
@@ -19,6 +19,7 @@ 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/hypervisor.h>
 
 #include "vmci_driver.h"
 #include "vmci_event.h"
@@ -58,6 +59,13 @@  static int __init vmci_drv_init(void)
 	int vmci_err;
 	int error;
 
+	/*
+	 * Check if we are running on VMware's hypervisor and bail out
+	 * if we are not.
+	 */
+	if (x86_hyper != &x86_hyper_vmware)
+		return -ENODEV;
+
 	vmci_err = vmci_event_init();
 	if (vmci_err < VMCI_SUCCESS) {
 		pr_err("Failed to initialize VMCIEvent (result=%d)\n",