Re: [PATCH] vsock: only load vmci transport on VMware hypervisor by default

[Stefan Hajnoczi]

On Thu, Aug 17, 2017 at 08:00:29AM +0000, Dexuan Cui wrote:
> 
> Without the patch, vmw_vsock_vmci_transport.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 can'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).

Thanks for sending this patch, vmci's MODULE_ALIAS_NETPROTO(PF_VSOCK) is
a problem for vhost_vsock.ko (the virtio host driver) too.  A host
userspace program can create a AF_VSOCK socket before vhost_vsock is
loaded.  The vmci transport will be unconditionally loaded and that's
not the right behavior.

Putting aside nested virtualization, I want to load the transport (vmci,
Hyper-V, vsock) for which there is paravirtualized hardware present
inside the guest.

It's a little tricker on the host side (doesn't matter for Hyper-V and
probably also doesn't for VMware) because the host-side driver is a
software device with no hardware backing it.  In KVM we assume the
vhost_vsock.ko kernel module will be loaded sufficiently early.

Things get trickier with nested virtualization because the VM might want
to talk to its host but also to its nested VMs.  The simple way of
fixing this would be to allow two transports loaded simultaneously and
route traffic destined to CID 2 to the host transport and all other
traffic to the guest transport.

Perhaps we should discuss these cases a bit more to figure out how to
avoid conflicts over MODULE_ALIAS_NETPROTO(PF_VSOCK).

> 
> The patch also adds a module parameter "skip_hypervisor_check" for
> vmw_vsock_vmci_transport.ko.
> 
> Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
> Cc: Alok Kataria <akataria@xxxxxxxxxx>
> Cc: Andy King <acking@xxxxxxxxxx>
> Cc: Adit Ranadive <aditr@xxxxxxxxxx>
> Cc: George Zhang <georgezhang@xxxxxxxxxx>
> Cc: Jorgen Hansen <jhansen@xxxxxxxxxx>
> Cc: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
> Cc: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Cc: Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>
> ---
>  net/vmw_vsock/Kconfig          |  2 +-
>  net/vmw_vsock/vmci_transport.c | 11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/net/vmw_vsock/Kconfig b/net/vmw_vsock/Kconfig
> index a24369d..3f52929 100644
> --- a/net/vmw_vsock/Kconfig
> +++ b/net/vmw_vsock/Kconfig
> @@ -17,7 +17,7 @@ config VSOCKETS
>  
>  config VMWARE_VMCI_VSOCKETS
>  	tristate "VMware VMCI transport for Virtual Sockets"
> -	depends on VSOCKETS && VMWARE_VMCI
> +	depends on VSOCKETS && VMWARE_VMCI && HYPERVISOR_GUEST
>  	help
>  	  This module implements a VMCI transport for Virtual Sockets.
>  
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 10ae782..c068873 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -16,6 +16,7 @@
>  #include <linux/types.h>
>  #include <linux/bitops.h>
>  #include <linux/cred.h>
> +#include <linux/hypervisor.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> @@ -73,6 +74,10 @@ struct vmci_transport_recv_pkt_info {
>  	struct vmci_transport_packet pkt;
>  };
>  
> +static bool skip_hypervisor_check;
> +module_param(skip_hypervisor_check, bool, 0444);
> +MODULE_PARM_DESC(hot_add, "If set, attempt to load on non-VMware platforms");
> +
>  static LIST_HEAD(vmci_transport_cleanup_list);
>  static DEFINE_SPINLOCK(vmci_transport_cleanup_lock);
>  static DECLARE_WORK(vmci_transport_cleanup_work, vmci_transport_cleanup);
> @@ -2085,6 +2090,12 @@ static int __init vmci_transport_init(void)
>  {
>  	int err;
>  
> +	/* Check if we are running on VMware's hypervisor and bail out
> +	 * if we are not.
> +	 */
> +	if (!skip_hypervisor_check && x86_hyper != &x86_hyper_vmware)
> +		return -ENODEV;
> +
>  	/* Create the datagram handle that we will use to send and receive all
>  	 * VSocket control messages for this context.
>  	 */
> -- 
> 2.7.4
>

Attachment: signature.asc
Description: PGP signature