| 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
| ||
|
Message-ID: <39b25002-f380-426b-ba18-9276d143a0db@linux.microsoft.com>
Date: Fri, 21 Feb 2025 14:38:00 -0800
From: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
To: Michael Kelley <mhklinux@...look.com>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
"eahariha@...ux.microsoft.com" <eahariha@...ux.microsoft.com>,
"mukeshrathor@...rosoft.com" <mukeshrathor@...rosoft.com>
Cc: "kys@...rosoft.com" <kys@...rosoft.com>,
"haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
"wei.liu@...nel.org" <wei.liu@...nel.org>,
"decui@...rosoft.com" <decui@...rosoft.com>,
"catalin.marinas@....com" <catalin.marinas@....com>,
"will@...nel.org" <will@...nel.org>, "tglx@...utronix.de"
<tglx@...utronix.de>, "mingo@...hat.com" <mingo@...hat.com>,
"bp@...en8.de" <bp@...en8.de>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
"joro@...tes.org" <joro@...tes.org>,
"robin.murphy@....com" <robin.murphy@....com>, "arnd@...db.de"
<arnd@...db.de>,
"jinankjain@...ux.microsoft.com" <jinankjain@...ux.microsoft.com>,
"muminulrussell@...il.com" <muminulrussell@...il.com>,
"skinsburskii@...ux.microsoft.com" <skinsburskii@...ux.microsoft.com>
Subject: Re: [PATCH v3 2/3] hyperv: Change hv_root_partition into a function
On 2/21/2025 12:58 PM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@...ux.microsoft.com> Sent: Friday, February 21, 2025 11:57 AM
>>
>> Introduce hv_curr_partition_type to store the partition type
>> as an enum.
>>
>> Right now this is limited to guest or root partition, but there will
>> be other kinds in future and the enum is easily extensible.
>>
>> Set up hv_curr_partition_type early in Hyper-V initialization with
>> hv_identify_partition_type(). hv_root_partition() just queries this
>> value, and shouldn't be called before that.
>>
>> Making this check into a function sets the stage for adding a config
>> option to gate the compilation of root partition code. In particular,
>> hv_root_partition() can be stubbed out always be false if root
>> partition support isn't desired.
>>
>> Signed-off-by: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
>> Reviewed-by: Easwar Hariharan <eahariha@...ux.microsoft.com>
>> ---
>> arch/arm64/hyperv/mshyperv.c | 2 ++
>> arch/x86/hyperv/hv_init.c | 10 +++++-----
>> arch/x86/kernel/cpu/mshyperv.c | 24 ++--------------------
>> drivers/clocksource/hyperv_timer.c | 4 ++--
>> drivers/hv/hv.c | 10 +++++-----
>> drivers/hv/hv_common.c | 32 ++++++++++++++++++++++++------
>> drivers/hv/vmbus_drv.c | 2 +-
>> drivers/iommu/hyperv-iommu.c | 4 ++--
>> include/asm-generic/mshyperv.h | 15 ++++++++++++--
>> 9 files changed, 58 insertions(+), 45 deletions(-)
>
> Reviewed-by: Michael Kelley <mhklinux@...look.com>
>
Thank you!
>>
>> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
>> index 29fcfd595f48..2265ea5ce5ad 100644
>> --- a/arch/arm64/hyperv/mshyperv.c
>> +++ b/arch/arm64/hyperv/mshyperv.c
>> @@ -61,6 +61,8 @@ static int __init hyperv_init(void)
>> ms_hyperv.features, ms_hyperv.priv_high, ms_hyperv.hints,
>> ms_hyperv.misc_features);
>>
>> + hv_identify_partition_type();
>> +
>> ret = hv_common_init();
>> if (ret)
>> return ret;
>> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
>> index 9be1446f5bd3..ddeb40930bc8 100644
>> --- a/arch/x86/hyperv/hv_init.c
>> +++ b/arch/x86/hyperv/hv_init.c
>> @@ -90,7 +90,7 @@ static int hv_cpu_init(unsigned int cpu)
>> return 0;
>>
>> hvp = &hv_vp_assist_page[cpu];
>> - if (hv_root_partition) {
>> + if (hv_root_partition()) {
>> /*
>> * For root partition we get the hypervisor provided VP assist
>> * page, instead of allocating a new page.
>> @@ -242,7 +242,7 @@ static int hv_cpu_die(unsigned int cpu)
>>
>> if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
>> union hv_vp_assist_msr_contents msr = { 0 };
>> - if (hv_root_partition) {
>> + if (hv_root_partition()) {
>> /*
>> * For root partition the VP assist page is mapped to
>> * hypervisor provided page, and thus we unmap the
>> @@ -317,7 +317,7 @@ static int hv_suspend(void)
>> union hv_x64_msr_hypercall_contents hypercall_msr;
>> int ret;
>>
>> - if (hv_root_partition)
>> + if (hv_root_partition())
>> return -EPERM;
>>
>> /*
>> @@ -518,7 +518,7 @@ void __init hyperv_init(void)
>> rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
>> hypercall_msr.enable = 1;
>>
>> - if (hv_root_partition) {
>> + if (hv_root_partition()) {
>> struct page *pg;
>> void *src;
>>
>> @@ -592,7 +592,7 @@ void __init hyperv_init(void)
>> * If we're running as root, we want to create our own PCI MSI domain.
>> * We can't set this in hv_pci_init because that would be too late.
>> */
>> - if (hv_root_partition)
>> + if (hv_root_partition())
>> x86_init.irqs.create_pci_msi_domain = hv_create_pci_msi_domain;
>> #endif
>>
>> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
>> index f285757618fc..4f01f424ea5b 100644
>> --- a/arch/x86/kernel/cpu/mshyperv.c
>> +++ b/arch/x86/kernel/cpu/mshyperv.c
>> @@ -33,8 +33,6 @@
>> #include <asm/numa.h>
>> #include <asm/svm.h>
>>
>> -/* Is Linux running as the root partition? */
>> -bool hv_root_partition;
>> /* Is Linux running on nested Microsoft Hypervisor */
>> bool hv_nested;
>> struct ms_hyperv_info ms_hyperv;
>> @@ -451,25 +449,7 @@ static void __init ms_hyperv_init_platform(void)
>> pr_debug("Hyper-V: max %u virtual processors, %u logical processors\n",
>> ms_hyperv.max_vp_index, ms_hyperv.max_lp_index);
>>
>> - /*
>> - * Check CPU management privilege.
>> - *
>> - * To mirror what Windows does we should extract CPU management
>> - * features and use the ReservedIdentityBit to detect if Linux is the
>> - * root partition. But that requires negotiating CPU management
>> - * interface (a process to be finalized). For now, use the privilege
>> - * flag as the indicator for running as root.
>> - *
>> - * Hyper-V should never specify running as root and as a Confidential
>> - * VM. But to protect against a compromised/malicious Hyper-V trying
>> - * to exploit root behavior to expose Confidential VM memory, ignore
>> - * the root partition setting if also a Confidential VM.
>> - */
>> - if ((ms_hyperv.priv_high & HV_CPU_MANAGEMENT) &&
>> - !(ms_hyperv.priv_high & HV_ISOLATION)) {
>> - hv_root_partition = true;
>> - pr_info("Hyper-V: running as root partition\n");
>> - }
>> + hv_identify_partition_type();
>>
>> if (ms_hyperv.hints & HV_X64_HYPERV_NESTED) {
>> hv_nested = true;
>> @@ -618,7 +598,7 @@ static void __init ms_hyperv_init_platform(void)
>>
>> # ifdef CONFIG_SMP
>> smp_ops.smp_prepare_boot_cpu = hv_smp_prepare_boot_cpu;
>> - if (hv_root_partition ||
>> + if (hv_root_partition() ||
>> (!ms_hyperv.paravisor_present && hv_isolation_type_snp()))
>> smp_ops.smp_prepare_cpus = hv_smp_prepare_cpus;
>> # endif
>> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
>> index f00019b078a7..09549451dd51 100644
>> --- a/drivers/clocksource/hyperv_timer.c
>> +++ b/drivers/clocksource/hyperv_timer.c
>> @@ -582,7 +582,7 @@ static void __init hv_init_tsc_clocksource(void)
>> * mapped.
>> */
>> tsc_msr.as_uint64 = hv_get_msr(HV_MSR_REFERENCE_TSC);
>> - if (hv_root_partition)
>> + if (hv_root_partition())
>> tsc_pfn = tsc_msr.pfn;
>> else
>> tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
>> @@ -627,7 +627,7 @@ void __init hv_remap_tsc_clocksource(void)
>> if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
>> return;
>>
>> - if (!hv_root_partition) {
>> + if (!hv_root_partition()) {
>> WARN(1, "%s: attempt to remap TSC page in guest partition\n",
>> __func__);
>> return;
>> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
>> index fab0690b5c41..a38f84548bc2 100644
>> --- a/drivers/hv/hv.c
>> +++ b/drivers/hv/hv.c
>> @@ -144,7 +144,7 @@ int hv_synic_alloc(void)
>> * Synic message and event pages are allocated by paravisor.
>> * Skip these pages allocation here.
>> */
>> - if (!ms_hyperv.paravisor_present && !hv_root_partition) {
>> + if (!ms_hyperv.paravisor_present && !hv_root_partition()) {
>> hv_cpu->synic_message_page =
>> (void *)get_zeroed_page(GFP_ATOMIC);
>> if (!hv_cpu->synic_message_page) {
>> @@ -272,7 +272,7 @@ void hv_synic_enable_regs(unsigned int cpu)
>> simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
>> simp.simp_enabled = 1;
>>
>> - if (ms_hyperv.paravisor_present || hv_root_partition) {
>> + if (ms_hyperv.paravisor_present || hv_root_partition()) {
>> /* Mask out vTOM bit. ioremap_cache() maps decrypted */
>> u64 base = (simp.base_simp_gpa << HV_HYP_PAGE_SHIFT) &
>> ~ms_hyperv.shared_gpa_boundary;
>> @@ -291,7 +291,7 @@ void hv_synic_enable_regs(unsigned int cpu)
>> siefp.as_uint64 = hv_get_msr(HV_MSR_SIEFP);
>> siefp.siefp_enabled = 1;
>>
>> - if (ms_hyperv.paravisor_present || hv_root_partition) {
>> + if (ms_hyperv.paravisor_present || hv_root_partition()) {
>> /* Mask out vTOM bit. ioremap_cache() maps decrypted */
>> u64 base = (siefp.base_siefp_gpa << HV_HYP_PAGE_SHIFT) &
>> ~ms_hyperv.shared_gpa_boundary;
>> @@ -367,7 +367,7 @@ void hv_synic_disable_regs(unsigned int cpu)
>> * addresses.
>> */
>> simp.simp_enabled = 0;
>> - if (ms_hyperv.paravisor_present || hv_root_partition) {
>> + if (ms_hyperv.paravisor_present || hv_root_partition()) {
>> iounmap(hv_cpu->synic_message_page);
>> hv_cpu->synic_message_page = NULL;
>> } else {
>> @@ -379,7 +379,7 @@ void hv_synic_disable_regs(unsigned int cpu)
>> siefp.as_uint64 = hv_get_msr(HV_MSR_SIEFP);
>> siefp.siefp_enabled = 0;
>>
>> - if (ms_hyperv.paravisor_present || hv_root_partition) {
>> + if (ms_hyperv.paravisor_present || hv_root_partition()) {
>> iounmap(hv_cpu->synic_event_page);
>> hv_cpu->synic_event_page = NULL;
>> } else {
>> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
>> index 5cf9894b9e79..3d9cfcfbc854 100644
>> --- a/drivers/hv/hv_common.c
>> +++ b/drivers/hv/hv_common.c
>> @@ -34,8 +34,11 @@
>> u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
>> EXPORT_SYMBOL_GPL(hv_current_partition_id);
>>
>> +enum hv_partition_type hv_curr_partition_type;
>> +EXPORT_SYMBOL_GPL(hv_curr_partition_type);
>> +
>> /*
>> - * hv_root_partition, ms_hyperv and hv_nested are defined here with other
>> + * ms_hyperv and hv_nested are defined here with other
>> * Hyper-V specific globals so they are shared across all architectures and are
>> * built only when CONFIG_HYPERV is defined. But on x86,
>> * ms_hyperv_init_platform() is built even when CONFIG_HYPERV is not
>> @@ -43,9 +46,6 @@ EXPORT_SYMBOL_GPL(hv_current_partition_id);
>> * here, allowing for an overriding definition in the module containing
>> * ms_hyperv_init_platform().
>> */
>> -bool __weak hv_root_partition;
>> -EXPORT_SYMBOL_GPL(hv_root_partition);
>> -
>> bool __weak hv_nested;
>> EXPORT_SYMBOL_GPL(hv_nested);
>>
>> @@ -283,7 +283,7 @@ static void hv_kmsg_dump_register(void)
>>
>> static inline bool hv_output_page_exists(void)
>> {
>> - return hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE);
>> + return hv_root_partition() || IS_ENABLED(CONFIG_HYPERV_VTL_MODE);
>> }
>>
>> void __init hv_get_partition_id(void)
>> @@ -594,7 +594,7 @@ EXPORT_SYMBOL_GPL(hv_setup_dma_ops);
>>
>> bool hv_is_hibernation_supported(void)
>> {
>> - return !hv_root_partition && acpi_sleep_state_supported(ACPI_STATE_S4);
>> + return !hv_root_partition() && acpi_sleep_state_supported(ACPI_STATE_S4);
>> }
>> EXPORT_SYMBOL_GPL(hv_is_hibernation_supported);
>>
>> @@ -717,3 +717,23 @@ int hv_result_to_errno(u64 status)
>> }
>> return -EIO;
>> }
>> +
>> +void hv_identify_partition_type(void)
>> +{
>> + /* Assume guest role */
>> + hv_curr_partition_type = HV_PARTITION_TYPE_GUEST;
>> + /*
>> + * Check partition creation and cpu management privileges
>> + *
>> + * Hyper-V should never specify running as root and as a Confidential
>> + * VM. But to protect against a compromised/malicious Hyper-V trying
>> + * to exploit root behavior to expose Confidential VM memory, ignore
>> + * the root partition setting if also a Confidential VM.
>> + */
>> + if ((ms_hyperv.priv_high & HV_CREATE_PARTITIONS) &&
>> + (ms_hyperv.priv_high & HV_CPU_MANAGEMENT) &&
>> + !(ms_hyperv.priv_high & HV_ISOLATION)) {
>> + pr_info("Hyper-V: running as root partition\n");
>> + hv_curr_partition_type = HV_PARTITION_TYPE_ROOT;
>> + }
>> +}
>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>> index 75eb1390b45c..22afebfc28ff 100644
>> --- a/drivers/hv/vmbus_drv.c
>> +++ b/drivers/hv/vmbus_drv.c
>> @@ -2656,7 +2656,7 @@ static int __init hv_acpi_init(void)
>> if (!hv_is_hyperv_initialized())
>> return -ENODEV;
>>
>> - if (hv_root_partition && !hv_nested)
>> + if (hv_root_partition() && !hv_nested)
>> return 0;
>>
>> /*
>> diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
>> index 2a86aa5d54c6..53e4b37716af 100644
>> --- a/drivers/iommu/hyperv-iommu.c
>> +++ b/drivers/iommu/hyperv-iommu.c
>> @@ -130,7 +130,7 @@ static int __init hyperv_prepare_irq_remapping(void)
>> x86_init.hyper.msi_ext_dest_id())
>> return -ENODEV;
>>
>> - if (hv_root_partition) {
>> + if (hv_root_partition()) {
>> name = "HYPERV-ROOT-IR";
>> ops = &hyperv_root_ir_domain_ops;
>> } else {
>> @@ -151,7 +151,7 @@ static int __init hyperv_prepare_irq_remapping(void)
>> return -ENOMEM;
>> }
>>
>> - if (hv_root_partition)
>> + if (hv_root_partition())
>> return 0; /* The rest is only relevant to guests */
>>
>> /*
>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>> index 3f115e2bcdaa..54ebd630e72c 100644
>> --- a/include/asm-generic/mshyperv.h
>> +++ b/include/asm-generic/mshyperv.h
>> @@ -28,6 +28,11 @@
>>
>> #define VTPM_BASE_ADDRESS 0xfed40000
>>
>> +enum hv_partition_type {
>> + HV_PARTITION_TYPE_GUEST,
>> + HV_PARTITION_TYPE_ROOT,
>> +};
>> +
>> struct ms_hyperv_info {
>> u32 features;
>> u32 priv_high;
>> @@ -59,6 +64,7 @@ struct ms_hyperv_info {
>> extern struct ms_hyperv_info ms_hyperv;
>> extern bool hv_nested;
>> extern u64 hv_current_partition_id;
>> +extern enum hv_partition_type hv_curr_partition_type;
>>
>> extern void * __percpu *hyperv_pcpu_input_arg;
>> extern void * __percpu *hyperv_pcpu_output_arg;
>> @@ -190,8 +196,6 @@ void hv_remove_crash_handler(void);
>> extern int vmbus_interrupt;
>> extern int vmbus_irq;
>>
>> -extern bool hv_root_partition;
>> -
>> #if IS_ENABLED(CONFIG_HYPERV)
>> /*
>> * Hypervisor's notion of virtual processor ID is different from
>> @@ -213,6 +217,7 @@ void __init hv_common_free(void);
>> void __init ms_hyperv_late_init(void);
>> int hv_common_cpu_init(unsigned int cpu);
>> int hv_common_cpu_die(unsigned int cpu);
>> +void hv_identify_partition_type(void);
>>
>> void *hv_alloc_hyperv_page(void);
>> void *hv_alloc_hyperv_zeroed_page(void);
>> @@ -310,6 +315,7 @@ void hyperv_cleanup(void);
>> bool hv_query_ext_cap(u64 cap_query);
>> void hv_setup_dma_ops(struct device *dev, bool coherent);
>> #else /* CONFIG_HYPERV */
>> +static inline void hv_identify_partition_type(void) {}
>> static inline bool hv_is_hyperv_initialized(void) { return false; }
>> static inline bool hv_is_hibernation_supported(void) { return false; }
>> static inline void hyperv_cleanup(void) {}
>> @@ -321,4 +327,9 @@ static inline enum hv_isolation_type
>> hv_get_isolation_type(void)
>> }
>> #endif /* CONFIG_HYPERV */
>>
>> +static inline bool hv_root_partition(void)
>> +{
>> + return hv_curr_partition_type == HV_PARTITION_TYPE_ROOT;
>> +}
>> +
>> #endif
>> --
>> 2.34.1
Powered by blists - more mailing lists