[PATCH v2 0/4] Drivers: hv: vmbus: Restrict devices and configurations on 'isolated' guests

[PATCH v2 0/4] Drivers: hv: vmbus: Restrict devices and configurations on 'isolated' guests

[Andrea Parri (Microsoft)]

Changes since v1 [1]:
  - improve/add logging (Haiyang Zhang)
  - move NVSC version check after version negotiation (Haiyang Zhang)

[1] https://lkml.kernel.org/r/20210119175841.22248-1-parri.andrea@gmail.com

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: x86@kernel.org
Cc: linux-arch@vger.kernel.org
Cc: netdev@vger.kernel.org

Andrea Parri (Microsoft) (4):
  x86/hyperv: Load/save the Isolation Configuration leaf
  Drivers: hv: vmbus: Restrict vmbus_devices on isolated guests
  Drivers: hv: vmbus: Enforce 'VMBus version >= 5.2' on isolated guests
  hv_netvsc: Restrict configurations on isolated guests

 arch/x86/hyperv/hv_init.c          | 15 +++++++++++++
 arch/x86/include/asm/hyperv-tlfs.h | 15 +++++++++++++
 arch/x86/kernel/cpu/mshyperv.c     |  9 ++++++++
 drivers/hv/channel_mgmt.c          | 36 ++++++++++++++++++++++++++++++
 drivers/hv/connection.c            | 13 +++++++++++
 drivers/net/hyperv/netvsc.c        | 27 +++++++++++++++++++---
 include/asm-generic/hyperv-tlfs.h  |  1 +
 include/asm-generic/mshyperv.h     |  5 +++++
 include/linux/hyperv.h             |  1 +
 9 files changed, 119 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH v2 1/4] x86/hyperv: Load/save the Isolation Configuration leaf

[Andrea Parri (Microsoft)]

If bit 22 of Group B Features is set, the guest has access to the
Isolation Configuration CPUID leaf.  On x86, the first four bits
of EAX in this leaf provide the isolation type of the partition;
we entail three isolation types: 'SNP' (hardware-based isolation),
'VBS' (software-based isolation), and 'NONE' (no isolation).

Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: x86@kernel.org
Cc: linux-arch@vger.kernel.org
---
 arch/x86/hyperv/hv_init.c          | 15 +++++++++++++++
 arch/x86/include/asm/hyperv-tlfs.h | 15 +++++++++++++++
 arch/x86/kernel/cpu/mshyperv.c     |  9 +++++++++
 include/asm-generic/hyperv-tlfs.h  |  1 +
 include/asm-generic/mshyperv.h     |  5 +++++
 5 files changed, 45 insertions(+)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index e04d90af4c27c..dc94e95c57b98 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -10,6 +10,7 @@
 #include <linux/acpi.h>
 #include <linux/efi.h>
 #include <linux/types.h>
+#include <linux/bitfield.h>
 #include <asm/apic.h>
 #include <asm/desc.h>
 #include <asm/hypervisor.h>
@@ -528,3 +529,17 @@ bool hv_is_hibernation_supported(void)
 	return acpi_sleep_state_supported(ACPI_STATE_S4);
 }
 EXPORT_SYMBOL_GPL(hv_is_hibernation_supported);
+
+enum hv_isolation_type hv_get_isolation_type(void)
+{
+	if (!(ms_hyperv.hypercalls_features & HV_ISOLATION))
+		return HV_ISOLATION_TYPE_NONE;
+	return FIELD_GET(HV_ISOLATION_TYPE, ms_hyperv.isolation_config_b);
+}
+EXPORT_SYMBOL_GPL(hv_get_isolation_type);
+
+bool hv_is_isolation_supported(void)
+{
+	return hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE;
+}
+EXPORT_SYMBOL_GPL(hv_is_isolation_supported);
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 6bf42aed387e3..6aed936e5e962 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -22,6 +22,7 @@
 #define HYPERV_CPUID_ENLIGHTMENT_INFO		0x40000004
 #define HYPERV_CPUID_IMPLEMENT_LIMITS		0x40000005
 #define HYPERV_CPUID_NESTED_FEATURES		0x4000000A
+#define HYPERV_CPUID_ISOLATION_CONFIG		0x4000000C
 
 #define HYPERV_CPUID_VIRT_STACK_INTERFACE	0x40000081
 #define HYPERV_VS_INTERFACE_EAX_SIGNATURE	0x31235356  /* "VS#1" */
@@ -122,6 +123,20 @@
 #define HV_X64_NESTED_GUEST_MAPPING_FLUSH		BIT(18)
 #define HV_X64_NESTED_MSR_BITMAP			BIT(19)
 
+/* HYPERV_CPUID_ISOLATION_CONFIG.EAX bits. */
+#define HV_PARAVISOR_PRESENT				BIT(0)
+
+/* HYPERV_CPUID_ISOLATION_CONFIG.EBX bits. */
+#define HV_ISOLATION_TYPE				GENMASK(3, 0)
+#define HV_SHARED_GPA_BOUNDARY_ACTIVE			BIT(5)
+#define HV_SHARED_GPA_BOUNDARY_BITS			GENMASK(11, 6)
+
+enum hv_isolation_type {
+	HV_ISOLATION_TYPE_NONE	= 0,
+	HV_ISOLATION_TYPE_VBS	= 1,
+	HV_ISOLATION_TYPE_SNP	= 2
+};
+
 /* Hyper-V specific model specific registers (MSRs) */
 
 /* MSR used to identify the guest OS. */
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index f628e3dc150f3..0d4aaf6694d01 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -225,6 +225,7 @@ static void __init ms_hyperv_init_platform(void)
 	 * Extract the features and hints
 	 */
 	ms_hyperv.features = cpuid_eax(HYPERV_CPUID_FEATURES);
+	ms_hyperv.hypercalls_features = cpuid_ebx(HYPERV_CPUID_FEATURES);
 	ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
 	ms_hyperv.hints    = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
 
@@ -259,6 +260,14 @@ static void __init ms_hyperv_init_platform(void)
 		x86_platform.calibrate_cpu = hv_get_tsc_khz;
 	}
 
+	if (ms_hyperv.hypercalls_features & HV_ISOLATION) {
+		ms_hyperv.isolation_config_a = cpuid_eax(HYPERV_CPUID_ISOLATION_CONFIG);
+		ms_hyperv.isolation_config_b = cpuid_ebx(HYPERV_CPUID_ISOLATION_CONFIG);
+
+		pr_info("Hyper-V: Isolation Config: GroupA 0x%x, GroupB 0x%x\n",
+			ms_hyperv.isolation_config_a, ms_hyperv.isolation_config_b);
+	}
+
 	if (ms_hyperv.hints & HV_X64_ENLIGHTENED_VMCS_RECOMMENDED) {
 		ms_hyperv.nested_features =
 			cpuid_eax(HYPERV_CPUID_NESTED_FEATURES);
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index e73a11850055c..20d3cd9502043 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -89,6 +89,7 @@
 #define HV_ACCESS_STATS				BIT(8)
 #define HV_DEBUGGING				BIT(11)
 #define HV_CPU_POWER_MANAGEMENT			BIT(12)
+#define HV_ISOLATION				BIT(22)
 
 
 /*
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index c57799684170c..c7f75b36f88ba 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -27,11 +27,14 @@
 
 struct ms_hyperv_info {
 	u32 features;
+	u32 hypercalls_features;
 	u32 misc_features;
 	u32 hints;
 	u32 nested_features;
 	u32 max_vp_index;
 	u32 max_lp_index;
+	u32 isolation_config_a;
+	u32 isolation_config_b;
 };
 extern struct ms_hyperv_info ms_hyperv;
 
@@ -169,6 +172,8 @@ void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die);
 void hyperv_report_panic_msg(phys_addr_t pa, size_t size);
 bool hv_is_hyperv_initialized(void);
 bool hv_is_hibernation_supported(void);
+enum hv_isolation_type hv_get_isolation_type(void);
+bool hv_is_isolation_supported(void);
 void hyperv_cleanup(void);
 #else /* CONFIG_HYPERV */
 static inline bool hv_is_hyperv_initialized(void) { return false; }
-- 
2.25.1

RE: [PATCH v2 1/4] x86/hyperv: Load/save the Isolation Configuration leaf

[Michael Kelley]

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Tuesday, January 26, 2021 3:57 AM
> 
> If bit 22 of Group B Features is set, the guest has access to the
> Isolation Configuration CPUID leaf.  On x86, the first four bits
> of EAX in this leaf provide the isolation type of the partition;
> we entail three isolation types: 'SNP' (hardware-based isolation),
> 'VBS' (software-based isolation), and 'NONE' (no isolation).
> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: x86@kernel.org
> Cc: linux-arch@vger.kernel.org
> ---
>  arch/x86/hyperv/hv_init.c          | 15 +++++++++++++++
>  arch/x86/include/asm/hyperv-tlfs.h | 15 +++++++++++++++
>  arch/x86/kernel/cpu/mshyperv.c     |  9 +++++++++
>  include/asm-generic/hyperv-tlfs.h  |  1 +
>  include/asm-generic/mshyperv.h     |  5 +++++
>  5 files changed, 45 insertions(+)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index e04d90af4c27c..dc94e95c57b98 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -10,6 +10,7 @@
>  #include <linux/acpi.h>
>  #include <linux/efi.h>
>  #include <linux/types.h>
> +#include <linux/bitfield.h>
>  #include <asm/apic.h>
>  #include <asm/desc.h>
>  #include <asm/hypervisor.h>
> @@ -528,3 +529,17 @@ bool hv_is_hibernation_supported(void)
>  	return acpi_sleep_state_supported(ACPI_STATE_S4);
>  }
>  EXPORT_SYMBOL_GPL(hv_is_hibernation_supported);
> +
> +enum hv_isolation_type hv_get_isolation_type(void)
> +{
> +	if (!(ms_hyperv.hypercalls_features & HV_ISOLATION))
> +		return HV_ISOLATION_TYPE_NONE;
> +	return FIELD_GET(HV_ISOLATION_TYPE, ms_hyperv.isolation_config_b);
> +}
> +EXPORT_SYMBOL_GPL(hv_get_isolation_type);
> +
> +bool hv_is_isolation_supported(void)
> +{
> +	return hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE;
> +}
> +EXPORT_SYMBOL_GPL(hv_is_isolation_supported);
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index 6bf42aed387e3..6aed936e5e962 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -22,6 +22,7 @@
>  #define HYPERV_CPUID_ENLIGHTMENT_INFO		0x40000004
>  #define HYPERV_CPUID_IMPLEMENT_LIMITS		0x40000005
>  #define HYPERV_CPUID_NESTED_FEATURES		0x4000000A
> +#define HYPERV_CPUID_ISOLATION_CONFIG		0x4000000C
> 
>  #define HYPERV_CPUID_VIRT_STACK_INTERFACE	0x40000081
>  #define HYPERV_VS_INTERFACE_EAX_SIGNATURE	0x31235356  /* "VS#1" */
> @@ -122,6 +123,20 @@
>  #define HV_X64_NESTED_GUEST_MAPPING_FLUSH		BIT(18)
>  #define HV_X64_NESTED_MSR_BITMAP			BIT(19)
> 
> +/* HYPERV_CPUID_ISOLATION_CONFIG.EAX bits. */
> +#define HV_PARAVISOR_PRESENT				BIT(0)
> +
> +/* HYPERV_CPUID_ISOLATION_CONFIG.EBX bits. */
> +#define HV_ISOLATION_TYPE				GENMASK(3, 0)
> +#define HV_SHARED_GPA_BOUNDARY_ACTIVE			BIT(5)
> +#define HV_SHARED_GPA_BOUNDARY_BITS			GENMASK(11, 6)
> +
> +enum hv_isolation_type {
> +	HV_ISOLATION_TYPE_NONE	= 0,
> +	HV_ISOLATION_TYPE_VBS	= 1,
> +	HV_ISOLATION_TYPE_SNP	= 2
> +};
> +
>  /* Hyper-V specific model specific registers (MSRs) */
> 
>  /* MSR used to identify the guest OS. */
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index f628e3dc150f3..0d4aaf6694d01 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -225,6 +225,7 @@ static void __init ms_hyperv_init_platform(void)
>  	 * Extract the features and hints
>  	 */
>  	ms_hyperv.features = cpuid_eax(HYPERV_CPUID_FEATURES);
> +	ms_hyperv.hypercalls_features = cpuid_ebx(HYPERV_CPUID_FEATURES);
>  	ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
>  	ms_hyperv.hints    = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
> 
> @@ -259,6 +260,14 @@ static void __init ms_hyperv_init_platform(void)
>  		x86_platform.calibrate_cpu = hv_get_tsc_khz;
>  	}
> 
> +	if (ms_hyperv.hypercalls_features & HV_ISOLATION) {
> +		ms_hyperv.isolation_config_a = cpuid_eax(HYPERV_CPUID_ISOLATION_CONFIG);
> +		ms_hyperv.isolation_config_b = cpuid_ebx(HYPERV_CPUID_ISOLATION_CONFIG);
> +
> +		pr_info("Hyper-V: Isolation Config: GroupA 0x%x, GroupB 0x%x\n",

Nit:  Let's put a space after the word "Group", so that we have
"Hyper-V: Isolation Config: Group A 0x%x, Group B 0x%x\n".

> +			ms_hyperv.isolation_config_a, ms_hyperv.isolation_config_b);
> +	}
> +
>  	if (ms_hyperv.hints & HV_X64_ENLIGHTENED_VMCS_RECOMMENDED) {
>  		ms_hyperv.nested_features =
>  			cpuid_eax(HYPERV_CPUID_NESTED_FEATURES);
> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
> index e73a11850055c..20d3cd9502043 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -89,6 +89,7 @@
>  #define HV_ACCESS_STATS				BIT(8)
>  #define HV_DEBUGGING				BIT(11)
>  #define HV_CPU_POWER_MANAGEMENT			BIT(12)
> +#define HV_ISOLATION				BIT(22)
> 
> 
>  /*
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index c57799684170c..c7f75b36f88ba 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -27,11 +27,14 @@
> 
>  struct ms_hyperv_info {
>  	u32 features;
> +	u32 hypercalls_features;

There was an offline conversation about renaming some of these fields so
they more obviously map to the Hyper-V TLFS while also being architecture
neutral.  Toward that end, I'd suggest the "hypercalls_features" field should be
named "features_b".  This would align with the offline discussion, and matches
what you've done with "isolation_config_a"  and similar below (which is good).

>  	u32 misc_features;
>  	u32 hints;
>  	u32 nested_features;
>  	u32 max_vp_index;
>  	u32 max_lp_index;
> +	u32 isolation_config_a;
> +	u32 isolation_config_b;
>  };
>  extern struct ms_hyperv_info ms_hyperv;
> 
> @@ -169,6 +172,8 @@ void hyperv_report_panic(struct pt_regs *regs, long err, bool
> in_die);
>  void hyperv_report_panic_msg(phys_addr_t pa, size_t size);
>  bool hv_is_hyperv_initialized(void);
>  bool hv_is_hibernation_supported(void);
> +enum hv_isolation_type hv_get_isolation_type(void);
> +bool hv_is_isolation_supported(void);
>  void hyperv_cleanup(void);
>  #else /* CONFIG_HYPERV */
>  static inline bool hv_is_hyperv_initialized(void) { return false; }
> --
> 2.25.1

[PATCH v2 2/4] Drivers: hv: vmbus: Restrict vmbus_devices on isolated guests

[Andrea Parri (Microsoft)]

Only the VSCs or ICs that have been hardened and that are critical for
the successful adoption of Confidential VMs should be allowed if the
guest is running isolated.  This change reduces the footprint of the
code that will be exercised by Confidential VMs and hence the exposure
to bugs and vulnerabilities.

Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
 drivers/hv/channel_mgmt.c | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/hyperv.h    |  1 +
 2 files changed, 37 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 68950a1e4b638..774ee19e3e90d 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -31,101 +31,118 @@ const struct vmbus_device vmbus_devs[] = {
 	{ .dev_type = HV_IDE,
 	  HV_IDE_GUID,
 	  .perf_device = true,
+	  .allowed_in_isolated = false,
 	},
 
 	/* SCSI */
 	{ .dev_type = HV_SCSI,
 	  HV_SCSI_GUID,
 	  .perf_device = true,
+	  .allowed_in_isolated = true,
 	},
 
 	/* Fibre Channel */
 	{ .dev_type = HV_FC,
 	  HV_SYNTHFC_GUID,
 	  .perf_device = true,
+	  .allowed_in_isolated = false,
 	},
 
 	/* Synthetic NIC */
 	{ .dev_type = HV_NIC,
 	  HV_NIC_GUID,
 	  .perf_device = true,
+	  .allowed_in_isolated = true,
 	},
 
 	/* Network Direct */
 	{ .dev_type = HV_ND,
 	  HV_ND_GUID,
 	  .perf_device = true,
+	  .allowed_in_isolated = false,
 	},
 
 	/* PCIE */
 	{ .dev_type = HV_PCIE,
 	  HV_PCIE_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = false,
 	},
 
 	/* Synthetic Frame Buffer */
 	{ .dev_type = HV_FB,
 	  HV_SYNTHVID_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = false,
 	},
 
 	/* Synthetic Keyboard */
 	{ .dev_type = HV_KBD,
 	  HV_KBD_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = false,
 	},
 
 	/* Synthetic MOUSE */
 	{ .dev_type = HV_MOUSE,
 	  HV_MOUSE_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = false,
 	},
 
 	/* KVP */
 	{ .dev_type = HV_KVP,
 	  HV_KVP_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = false,
 	},
 
 	/* Time Synch */
 	{ .dev_type = HV_TS,
 	  HV_TS_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = true,
 	},
 
 	/* Heartbeat */
 	{ .dev_type = HV_HB,
 	  HV_HEART_BEAT_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = true,
 	},
 
 	/* Shutdown */
 	{ .dev_type = HV_SHUTDOWN,
 	  HV_SHUTDOWN_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = true,
 	},
 
 	/* File copy */
 	{ .dev_type = HV_FCOPY,
 	  HV_FCOPY_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = false,
 	},
 
 	/* Backup */
 	{ .dev_type = HV_BACKUP,
 	  HV_VSS_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = false,
 	},
 
 	/* Dynamic Memory */
 	{ .dev_type = HV_DM,
 	  HV_DM_GUID,
 	  .perf_device = false,
+	  .allowed_in_isolated = false,
 	},
 
 	/* Unknown GUID */
 	{ .dev_type = HV_UNKNOWN,
 	  .perf_device = false,
+	  .allowed_in_isolated = false,
 	},
 };
 
@@ -903,6 +920,20 @@ find_primary_channel_by_offer(const struct vmbus_channel_offer_channel *offer)
 	return channel;
 }
 
+static bool vmbus_is_valid_device(const guid_t *guid)
+{
+	u16 i;
+
+	if (!hv_is_isolation_supported())
+		return true;
+
+	for (i = 0; i < ARRAY_SIZE(vmbus_devs); i++) {
+		if (guid_equal(guid, &vmbus_devs[i].guid))
+			return vmbus_devs[i].allowed_in_isolated;
+	}
+	return false;
+}
+
 /*
  * vmbus_onoffer - Handler for channel offers from vmbus in parent partition.
  *
@@ -917,6 +948,11 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
 
 	trace_vmbus_onoffer(offer);
 
+	if (!vmbus_is_valid_device(&offer->offer.if_type)) {
+		atomic_dec(&vmbus_connection.offer_in_progress);
+		return;
+	}
+
 	oldchannel = find_primary_channel_by_offer(offer);
 
 	if (oldchannel != NULL) {
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index f0d48a368f131..e3426f8c12db9 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -789,6 +789,7 @@ struct vmbus_device {
 	u16  dev_type;
 	guid_t guid;
 	bool perf_device;
+	bool allowed_in_isolated;
 };
 
 #define VMBUS_DEFAULT_MAX_PKT_SIZE 4096
-- 
2.25.1

RE: [PATCH v2 2/4] Drivers: hv: vmbus: Restrict vmbus_devices on isolated guests

[Michael Kelley]

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Tuesday, January 26, 2021 3:57 AM
> 
> Only the VSCs or ICs that have been hardened and that are critical for
> the successful adoption of Confidential VMs should be allowed if the
> guest is running isolated.  This change reduces the footprint of the
> code that will be exercised by Confidential VMs and hence the exposure
> to bugs and vulnerabilities.
> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
>  drivers/hv/channel_mgmt.c | 36 ++++++++++++++++++++++++++++++++++++
>  include/linux/hyperv.h    |  1 +
>  2 files changed, 37 insertions(+)
> 
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 68950a1e4b638..774ee19e3e90d 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -31,101 +31,118 @@ const struct vmbus_device vmbus_devs[] = {
>  	{ .dev_type = HV_IDE,
>  	  HV_IDE_GUID,
>  	  .perf_device = true,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* SCSI */
>  	{ .dev_type = HV_SCSI,
>  	  HV_SCSI_GUID,
>  	  .perf_device = true,
> +	  .allowed_in_isolated = true,
>  	},
> 
>  	/* Fibre Channel */
>  	{ .dev_type = HV_FC,
>  	  HV_SYNTHFC_GUID,
>  	  .perf_device = true,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* Synthetic NIC */
>  	{ .dev_type = HV_NIC,
>  	  HV_NIC_GUID,
>  	  .perf_device = true,
> +	  .allowed_in_isolated = true,
>  	},
> 
>  	/* Network Direct */
>  	{ .dev_type = HV_ND,
>  	  HV_ND_GUID,
>  	  .perf_device = true,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* PCIE */
>  	{ .dev_type = HV_PCIE,
>  	  HV_PCIE_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* Synthetic Frame Buffer */
>  	{ .dev_type = HV_FB,
>  	  HV_SYNTHVID_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* Synthetic Keyboard */
>  	{ .dev_type = HV_KBD,
>  	  HV_KBD_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* Synthetic MOUSE */
>  	{ .dev_type = HV_MOUSE,
>  	  HV_MOUSE_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* KVP */
>  	{ .dev_type = HV_KVP,
>  	  HV_KVP_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* Time Synch */
>  	{ .dev_type = HV_TS,
>  	  HV_TS_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = true,
>  	},
> 
>  	/* Heartbeat */
>  	{ .dev_type = HV_HB,
>  	  HV_HEART_BEAT_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = true,
>  	},
> 
>  	/* Shutdown */
>  	{ .dev_type = HV_SHUTDOWN,
>  	  HV_SHUTDOWN_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = true,
>  	},
> 
>  	/* File copy */
>  	{ .dev_type = HV_FCOPY,
>  	  HV_FCOPY_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* Backup */
>  	{ .dev_type = HV_BACKUP,
>  	  HV_VSS_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* Dynamic Memory */
>  	{ .dev_type = HV_DM,
>  	  HV_DM_GUID,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = false,
>  	},
> 
>  	/* Unknown GUID */
>  	{ .dev_type = HV_UNKNOWN,
>  	  .perf_device = false,
> +	  .allowed_in_isolated = false,
>  	},
>  };
> 
> @@ -903,6 +920,20 @@ find_primary_channel_by_offer(const struct
> vmbus_channel_offer_channel *offer)
>  	return channel;
>  }
> 
> +static bool vmbus_is_valid_device(const guid_t *guid)
> +{
> +	u16 i;
> +
> +	if (!hv_is_isolation_supported())
> +		return true;
> +
> +	for (i = 0; i < ARRAY_SIZE(vmbus_devs); i++) {
> +		if (guid_equal(guid, &vmbus_devs[i].guid))
> +			return vmbus_devs[i].allowed_in_isolated;
> +	}
> +	return false;
> +}
> +
>  /*
>   * vmbus_onoffer - Handler for channel offers from vmbus in parent partition.
>   *
> @@ -917,6 +948,11 @@ static void vmbus_onoffer(struct vmbus_channel_message_header
> *hdr)
> 
>  	trace_vmbus_onoffer(offer);
> 
> +	if (!vmbus_is_valid_device(&offer->offer.if_type)) {

Output a message in this case?  It could be useful to know that an
offer has been dropped.  It might make sense to rate limit the message
like in some previous patches that are doing VMbus hardening.

> +		atomic_dec(&vmbus_connection.offer_in_progress);
> +		return;
> +	}
> +
>  	oldchannel = find_primary_channel_by_offer(offer);
> 
>  	if (oldchannel != NULL) {
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index f0d48a368f131..e3426f8c12db9 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -789,6 +789,7 @@ struct vmbus_device {
>  	u16  dev_type;
>  	guid_t guid;
>  	bool perf_device;
> +	bool allowed_in_isolated;
>  };
> 
>  #define VMBUS_DEFAULT_MAX_PKT_SIZE 4096
> --
> 2.25.1

[PATCH v2 3/4] Drivers: hv: vmbus: Enforce 'VMBus version >= 5.2' on isolated guests

[Andrea Parri (Microsoft)]

Restrict the protocol version(s) that will be negotiated with the host
to be 5.2 or greater if the guest is running isolated.  This reduces the
footprint of the code that will be exercised by Confidential VMs and
hence the exposure to bugs and vulnerabilities.

Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
 drivers/hv/connection.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 11170d9a2e1a5..bcf4d7def6838 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -66,6 +66,13 @@ module_param(max_version, uint, S_IRUGO);
 MODULE_PARM_DESC(max_version,
 		 "Maximal VMBus protocol version which can be negotiated");
 
+static bool vmbus_is_valid_version(u32 version)
+{
+	if (hv_is_isolation_supported())
+		return version >= VERSION_WIN10_V5_2;
+	return true;
+}
+
 int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
 {
 	int ret = 0;
@@ -233,6 +240,12 @@ int vmbus_connect(void)
 			goto cleanup;
 
 		version = vmbus_versions[i];
+
+		if (!vmbus_is_valid_version(version)) {
+			ret = -EINVAL;
+			goto cleanup;
+		}
+
 		if (version > max_version)
 			continue;
 
-- 
2.25.1

RE: [PATCH v2 3/4] Drivers: hv: vmbus: Enforce 'VMBus version >= 5.2' on isolated guests

[Michael Kelley]

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Tuesday, January 26, 2021 3:57 AM
> 
> Restrict the protocol version(s) that will be negotiated with the host
> to be 5.2 or greater if the guest is running isolated.  This reduces the
> footprint of the code that will be exercised by Confidential VMs and
> hence the exposure to bugs and vulnerabilities.
> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
>  drivers/hv/connection.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 11170d9a2e1a5..bcf4d7def6838 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -66,6 +66,13 @@ module_param(max_version, uint, S_IRUGO);
>  MODULE_PARM_DESC(max_version,
>  		 "Maximal VMBus protocol version which can be negotiated");
> 
> +static bool vmbus_is_valid_version(u32 version)
> +{
> +	if (hv_is_isolation_supported())
> +		return version >= VERSION_WIN10_V5_2;
> +	return true;
> +}
> +
>  int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
>  {
>  	int ret = 0;
> @@ -233,6 +240,12 @@ int vmbus_connect(void)
>  			goto cleanup;
> 
>  		version = vmbus_versions[i];
> +
> +		if (!vmbus_is_valid_version(version)) {

Outputting a message in this case could be useful.  The message should show
what version was negotiated and then deemed invalid. 

> +			ret = -EINVAL;
> +			goto cleanup;
> +		}
> +
>  		if (version > max_version)
>  			continue;
> 
> --
> 2.25.1

[PATCH v2 4/4] hv_netvsc: Restrict configurations on isolated guests

[Andrea Parri (Microsoft)]

Restrict the NVSP protocol version(s) that will be negotiated with the
host to be NVSP_PROTOCOL_VERSION_61 or greater if the guest is running
isolated.  Moreover, do not advertise the SR-IOV capability and ignore
NVSP_MSG_4_TYPE_SEND_VF_ASSOCIATION messages in isolated guests, which
are not supposed to support SR-IOV.  This reduces the footprint of the
code that will be exercised by Confidential VMs and hence the exposure
to bugs and vulnerabilities.

Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
---
 drivers/net/hyperv/netvsc.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 1510a236aa341..afd92b4aa21fe 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -22,6 +22,7 @@
 #include <linux/prefetch.h>
 
 #include <asm/sync_bitops.h>
+#include <asm/mshyperv.h>
 
 #include "hyperv_net.h"
 #include "netvsc_trace.h"
@@ -544,7 +545,10 @@ static int negotiate_nvsp_ver(struct hv_device *device,
 	init_packet->msg.v2_msg.send_ndis_config.capability.ieee8021q = 1;
 
 	if (nvsp_ver >= NVSP_PROTOCOL_VERSION_5) {
-		init_packet->msg.v2_msg.send_ndis_config.capability.sriov = 1;
+		if (!hv_is_isolation_supported())
+			init_packet->msg.v2_msg.send_ndis_config.capability.sriov = 1;
+		else
+			netdev_info(ndev, "SR-IOV not advertised by guests on the host supporting isolation\n");
 
 		/* Teaming bit is needed to receive link speed updates */
 		init_packet->msg.v2_msg.send_ndis_config.capability.teaming = 1;
@@ -563,6 +567,13 @@ static int negotiate_nvsp_ver(struct hv_device *device,
 	return ret;
 }
 
+static bool nvsp_is_valid_version(u32 version)
+{
+       if (hv_is_isolation_supported())
+               return version >= NVSP_PROTOCOL_VERSION_61;
+       return true;
+}
+
 static int netvsc_connect_vsp(struct hv_device *device,
 			      struct netvsc_device *net_device,
 			      const struct netvsc_device_info *device_info)
@@ -579,12 +590,19 @@ static int netvsc_connect_vsp(struct hv_device *device,
 	init_packet = &net_device->channel_init_pkt;
 
 	/* Negotiate the latest NVSP protocol supported */
-	for (i = ARRAY_SIZE(ver_list) - 1; i >= 0; i--)
+	for (i = ARRAY_SIZE(ver_list) - 1; i >= 0; i--) {
 		if (negotiate_nvsp_ver(device, net_device, init_packet,
 				       ver_list[i])  == 0) {
+			if (!nvsp_is_valid_version(ver_list[i])) {
+				netdev_err(ndev, "Invalid NVSP version 0x%x (expected >= 0x%x) from the host with isolation supported\n",
+					   ver_list[i], NVSP_PROTOCOL_VERSION_61);
+				ret = -EPROTO;
+				goto cleanup;
+			}
 			net_device->nvsp_version = ver_list[i];
 			break;
 		}
+	}
 
 	if (i < 0) {
 		ret = -EPROTO;
@@ -1357,7 +1375,10 @@ static void netvsc_receive_inband(struct net_device *ndev,
 		break;
 
 	case NVSP_MSG4_TYPE_SEND_VF_ASSOCIATION:
-		netvsc_send_vf(ndev, nvmsg, msglen);
+		if (!hv_is_isolation_supported())
+			netvsc_send_vf(ndev, nvmsg, msglen);
+		else
+			netdev_err(ndev, "Ignore VF_ASSOCIATION msg from the host supporting isolation\n");
 		break;
 	}
 }
-- 
2.25.1

RE: [PATCH v2 4/4] hv_netvsc: Restrict configurations on isolated guests

[Haiyang Zhang]

> -----Original Message-----
> From: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Sent: Tuesday, January 26, 2021 6:57 AM
> To: linux-kernel@vger.kernel.org
> Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; Wei Liu <wei.liu@kernel.org>; Michael Kelley
> <mikelley@microsoft.com>; linux-hyperv@vger.kernel.org; Tianyu Lan
> <Tianyu.Lan@microsoft.com>; Saruhan Karademir
> <skarade@microsoft.com>; Juan Vazquez <juvazq@microsoft.com>; Andrea
> Parri (Microsoft) <parri.andrea@gmail.com>; Jakub Kicinski
> <kuba@kernel.org>; David S. Miller <davem@davemloft.net>;
> netdev@vger.kernel.org
> Subject: [PATCH v2 4/4] hv_netvsc: Restrict configurations on isolated guests
> 
> Restrict the NVSP protocol version(s) that will be negotiated with the host to
> be NVSP_PROTOCOL_VERSION_61 or greater if the guest is running isolated.
> Moreover, do not advertise the SR-IOV capability and ignore
> NVSP_MSG_4_TYPE_SEND_VF_ASSOCIATION messages in isolated guests,
> which are not supposed to support SR-IOV.  This reduces the footprint of the
> code that will be exercised by Confidential VMs and hence the exposure to
> bugs and vulnerabilities.
> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org

Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Thanks.

RE: [PATCH v2 4/4] hv_netvsc: Restrict configurations on isolated guests

[Michael Kelley]

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Tuesday, January 26, 2021 3:57 AM
> 
> Restrict the NVSP protocol version(s) that will be negotiated with the
> host to be NVSP_PROTOCOL_VERSION_61 or greater if the guest is running
> isolated.  Moreover, do not advertise the SR-IOV capability and ignore
> NVSP_MSG_4_TYPE_SEND_VF_ASSOCIATION messages in isolated guests, which
> are not supposed to support SR-IOV.  This reduces the footprint of the
> code that will be exercised by Confidential VMs and hence the exposure
> to bugs and vulnerabilities.
> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/hyperv/netvsc.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 1510a236aa341..afd92b4aa21fe 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -22,6 +22,7 @@
>  #include <linux/prefetch.h>
> 
>  #include <asm/sync_bitops.h>
> +#include <asm/mshyperv.h>
> 
>  #include "hyperv_net.h"
>  #include "netvsc_trace.h"
> @@ -544,7 +545,10 @@ static int negotiate_nvsp_ver(struct hv_device *device,
>  	init_packet->msg.v2_msg.send_ndis_config.capability.ieee8021q = 1;
> 
>  	if (nvsp_ver >= NVSP_PROTOCOL_VERSION_5) {
> -		init_packet->msg.v2_msg.send_ndis_config.capability.sriov = 1;
> +		if (!hv_is_isolation_supported())
> +			init_packet->msg.v2_msg.send_ndis_config.capability.sriov = 1;
> +		else
> +			netdev_info(ndev, "SR-IOV not advertised by guests on the host supporting isolation\n");

Nit:  Flip the "if" and "else" clauses so that the ! isn't needed in the test.

> 
>  		/* Teaming bit is needed to receive link speed updates */
>  		init_packet->msg.v2_msg.send_ndis_config.capability.teaming = 1;
> @@ -563,6 +567,13 @@ static int negotiate_nvsp_ver(struct hv_device *device,
>  	return ret;
>  }
> 
> +static bool nvsp_is_valid_version(u32 version)
> +{
> +       if (hv_is_isolation_supported())
> +               return version >= NVSP_PROTOCOL_VERSION_61;
> +       return true;
> +}
> +
>  static int netvsc_connect_vsp(struct hv_device *device,
>  			      struct netvsc_device *net_device,
>  			      const struct netvsc_device_info *device_info)
> @@ -579,12 +590,19 @@ static int netvsc_connect_vsp(struct hv_device *device,
>  	init_packet = &net_device->channel_init_pkt;
> 
>  	/* Negotiate the latest NVSP protocol supported */
> -	for (i = ARRAY_SIZE(ver_list) - 1; i >= 0; i--)
> +	for (i = ARRAY_SIZE(ver_list) - 1; i >= 0; i--) {
>  		if (negotiate_nvsp_ver(device, net_device, init_packet,
>  				       ver_list[i])  == 0) {
> +			if (!nvsp_is_valid_version(ver_list[i])) {

Could this test go after the 'for' loop, like the test for i < 0?  That would
get the code unindented a lot.  And maybe the helper function logic
(i.e., nvsp_is_valid_version) could just be coded inline.

> +				netdev_err(ndev, "Invalid NVSP version 0x%x (expected >= 0x%x) from the host with isolation supported\n",

Nit: The other two new messages use the phrase "... the host supporting isolation".

> +					   ver_list[i], NVSP_PROTOCOL_VERSION_61);
> +				ret = -EPROTO;
> +				goto cleanup;
> +			}
>  			net_device->nvsp_version = ver_list[i];
>  			break;
>  		}
> +	}
> 
>  	if (i < 0) {
>  		ret = -EPROTO;
> @@ -1357,7 +1375,10 @@ static void netvsc_receive_inband(struct net_device *ndev,
>  		break;
> 
>  	case NVSP_MSG4_TYPE_SEND_VF_ASSOCIATION:
> -		netvsc_send_vf(ndev, nvmsg, msglen);
> +		if (!hv_is_isolation_supported())
> +			netvsc_send_vf(ndev, nvmsg, msglen);
> +		else
> +			netdev_err(ndev, "Ignore VF_ASSOCIATION msg from the host supporting isolation\n");

Nit:  Flip the "if" and "else" clauses so that the ! isn't needed in the test.

>  		break;
>  	}
>  }
> --
> 2.25.1