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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BL0PR2101MB0930CF4297121B1BB904AA7DCAA29@BL0PR2101MB0930.namprd21.prod.outlook.com>
Date:   Wed, 20 Jan 2021 19:24:05 +0000
From:   Haiyang Zhang <haiyangz@...rosoft.com>
To:     "Andrea Parri (Microsoft)" <parri.andrea@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     KY Srinivasan <kys@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>,
        Michael Kelley <mikelley@...rosoft.com>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>,
        Saruhan Karademir <skarade@...rosoft.com>,
        Juan Vazquez <juvazq@...rosoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH 4/4] hv_netvsc: Restrict configurations on isolated guests



> -----Original Message-----
> From: Andrea Parri (Microsoft) <parri.andrea@...il.com>
> Sent: Tuesday, January 19, 2021 12:59 PM
> To: linux-kernel@...r.kernel.org
> Cc: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> <haiyangz@...rosoft.com>; Stephen Hemminger
> <sthemmin@...rosoft.com>; Wei Liu <wei.liu@...nel.org>; Michael Kelley
> <mikelley@...rosoft.com>; Tianyu Lan <Tianyu.Lan@...rosoft.com>;
> Saruhan Karademir <skarade@...rosoft.com>; Juan Vazquez
> <juvazq@...rosoft.com>; linux-hyperv@...r.kernel.org; Andrea Parri
> (Microsoft) <parri.andrea@...il.com>; David S. Miller
> <davem@...emloft.net>; Jakub Kicinski <kuba@...nel.org>;
> netdev@...r.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@...il.com>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: netdev@...r.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-
> >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.

> 
>  		/* Teaming bit is needed to receive link speed updates */
>  		init_packet-
> >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.


> +}
> +
>  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).

>  		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