linux-kernel - RE: [PATCH] x86/hyperv: Use per cpu initial stack for vtl context

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:
 <SN6PR02MB415750EE9132ECB049D7B20AD4432@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Thu, 1 Feb 2024 19:53:59 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Saurabh Sengar <ssengar@...ux.microsoft.com>, "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>, "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>, "dwmw@...zon.co.uk"
	<dwmw@...zon.co.uk>, "peterz@...radead.org" <peterz@...radead.org>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "ssengar@...rosoft.com" <ssengar@...rosoft.com>
Subject: RE: [PATCH] x86/hyperv: Use per cpu initial stack for vtl context

From: Saurabh Sengar <ssengar@...ux.microsoft.com> Sent: Thursday, February 1, 2024 10:26 AM
> 
> Currently, the secondary vCPUs in Hyper-V VTL context lack support for
> parallel startup. Therefore, relying on the single initial_stack fetched
> from the current task structure suffices for all vCPUs.
> 
> However, common initial_stack risks stack corruption when parallel startup
> is enabled. In order to facilitate parallel startup, use the initial_stack
> from the per CPU idle thread instead of the current task.
> 
> Fixes: 18415f33e2ac ("cpu/hotplug: Allow "parallel" bringup up to CPUHP_BP_KICK_AP_STATE")
> Signed-off-by: Saurabh Sengar <ssengar@...ux.microsoft.com>
> ---
>  arch/x86/hyperv/hv_vtl.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 96e6c51515f5..a54b46b673de 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -12,6 +12,7 @@
>  #include <asm/i8259.h>
>  #include <asm/mshyperv.h>
>  #include <asm/realmode.h>
> +#include <../kernel/smpboot.h>
> 
>  extern struct boot_params boot_params;
>  static struct real_mode_header hv_vtl_real_mode_header;
> @@ -71,7 +72,8 @@ static int hv_vtl_bringup_vcpu(u32 target_vp_index, u64 eip_ignored)
>  	struct ldttss_desc *ldt;
>  	struct desc_struct *gdt;
> 
> -	u64 rsp = current->thread.sp;
> +	struct task_struct *idle = idle_thread_get(target_vp_index);

The "VP index" of a vCPU is a Hyper-V concept, and may not match
the Linux concept of a CPU number.   In most cases, they *do* match,
so your testing of this patch probably worked.  But there's no guarantee
that they match.  The Hyper-V TLFS does not even guarantee that VP
indices are dense or that they start with 0 (even though they do in
current versions of Hyper-V).

As a different kind of example, in a kdump kernel, Linux labels the
booting CPU as CPU 0, but it may not be the 0th CPU in the guest
VM, and hence may not have VP index of 0.  Of course, in a kdump
kernel nr_cpus is typically 1, so you aren't bringing up secondary
CPUs.  But sometimes kdump kernels boot with nr_cpus=2 or greater,
in which case the mismatch would occur.

This conceptual difference in VP index and Linux CPU numbers is why
the hv_vp_index array exists -- to map from a Linux CPU number to a
Hyper-V VP index, and thereby avoid assuming they are equal.

So before hv_vtl_wakeup_secondary_cpu() calls this function, it needs
to separately map the apicid to a Linux CPU number, which can then
be passed to idle_thread_get().

Michael

> +	u64 rsp = (unsigned long)idle->thread.sp;
>  	u64 rip = (u64)&hv_vtl_ap_entry;
> 
>  	native_store_gdt(&gdt_ptr);
> --
> 2.34.1
>