linux-kernel - RE: [PATCH v2 2/4] Drivers: hv: vmbus: Restrict vmbus

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]
Date:   Fri, 29 Jan 2021 00:35:49 +0000
From:   Michael Kelley <mikelley@...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>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>,
        Saruhan Karademir <skarade@...rosoft.com>,
        Juan Vazquez <juvazq@...rosoft.com>
Subject: RE: [PATCH v2 2/4] Drivers: hv: vmbus: Restrict vmbus_devices on
 isolated guests

From: Andrea Parri (Microsoft) <parri.andrea@...il.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@...il.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