Thread (7 messages) flat view 7 messages, 3 authors, 2021-01-21

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

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: 2021-01-20 19:27:24
Also in: linux-hyperv, lkml

quoted hunk ↗ jump to hunk
-----Original Message-----
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Sent: Tuesday, January 19, 2021 12:59 PM
To: linux-kernel@vger.kernel.org
Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
[off-list ref]; Stephen Hemminger
[off-list ref]; Wei Liu [off-list ref]; Michael Kelley
[off-list ref]; Tianyu Lan [off-list ref];
Saruhan Karademir [off-list ref]; Juan Vazquez
[off-list ref]; linux-hyperv@vger.kernel.org; Andrea Parri
(Microsoft) [off-list ref]; David S. Miller
[off-list ref]; Jakub Kicinski [off-list ref];
netdev@vger.kernel.org
Subject: [PATCH 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>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
---
 drivers/net/hyperv/netvsc.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 1510a236aa341..8027d553cb67d 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,8 @@ 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-
quoted
msg.v2_msg.send_ndis_config.capability.sriov = 1;
Please also add a log there stating we don't support sriov in this case. Otherwise,
customers will ask why vf not showing up.
quoted hunk ↗ jump to hunk
 		/* Teaming bit is needed to receive link speed updates */
 		init_packet-
quoted
msg.v2_msg.send_ndis_config.capability.teaming = 1;
@@ -563,6 +565,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;
Hosts support isolation should run nvsp 6.1+. This error is not expected.
Instead of fail silently, we should log an error to explain why it's failed, and the current version and expected version.

quoted hunk ↗ jump to hunk
+}
+
 static int netvsc_connect_vsp(struct hv_device *device,
 			      struct netvsc_device *net_device,
 			      const struct netvsc_device_info *device_info)
@@ -579,12 +588,17 @@ 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 (!nvsp_is_valid_version(ver_list[i])) {
+			ret = -EPROTO;
+			goto cleanup;
+		}
This code can catch the invalid, but cannot get the current host nvsp version.
I'd suggest move this check after version negotiation is done. So we can log what's
the current host nvsp version, and why we fail it (the expected nvsp ver).
quoted hunk ↗ jump to hunk
 		if (negotiate_nvsp_ver(device, net_device, init_packet,
 				       ver_list[i])  == 0) {
 			net_device->nvsp_version = ver_list[i];
 			break;
 		}
+	}

 	if (i < 0) {
 		ret = -EPROTO;
@@ -1357,7 +1371,8 @@ 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);
When the driver doesn't advertise SRIOV, this message is not expected.
Instead of ignore silently, we should log an error.

Thanks,
- Haiyang
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help