Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Message-ID: <0e86d1cb-a042-7d80-b410-0cc4b31744aa@grsecurity.net>
Date:   Thu, 21 Sep 2023 09:17:24 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.14.0
Subject: Re: [PATCH] x86/hyperv: Remove hv_vtl_early_init initcall
Content-Language: en-US, de-DE
To:     Saurabh Sengar <ssengar@linux.microsoft.com>, kys@microsoft.com,
        haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
        linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
        mikelley@microsoft.com
Cc:     ssengar@microsoft.com
References: <1695185552-19910-1-git-send-email-ssengar@linux.microsoft.com>
From:   Mathias Krause <minipli@grsecurity.net>
In-Reply-To: <1695185552-19910-1-git-send-email-ssengar@linux.microsoft.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 20.09.23 06:52, Saurabh Sengar wrote:
> There has been cases reported where HYPERV_VTL_MODE is enabled by mistake,
> on a non Hyper-V platforms. This causes the hv_vtl_early_init function to
> be called in an non Hyper-V/VTL platforms which results the memory
> corruption.
> 
> Remove the early_initcall for vhv_vtl_early_init and call it at the end of
> hyperv_init to make sure it is never called in a non Hyper-V platform by
> mistake.
> 
> Reported-by: Mathias Krause <minipli@grsecurity.net>
> Closes: https://lore.kernel.org/lkml/40467722-f4ab-19a5-4989-308225b1f9f0@grsecurity.net/
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
> This patch is dependent on :
> https://lore.kernel.org/lkml/1695182675-13405-1-git-send-email-ssengar@linux.microsoft.com/
> 
>  arch/x86/hyperv/hv_init.c       | 3 +++
>  arch/x86/hyperv/hv_vtl.c        | 3 +--
>  arch/x86/include/asm/mshyperv.h | 2 ++
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index f0128fd4031d..608f4fe41fb7 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -610,6 +610,9 @@ void __init hyperv_init(void)
>  	/* Find the VTL */
>  	ms_hyperv.vtl = get_vtl();
>  
> +	if (ms_hyperv.vtl > 0) /* non default VTL */
> +		hv_vtl_early_init();
> +

As the kernel's console will already be initialized when this code gets
executed, the possible panic() in hv_vtl_early_init() will actually be
visible, thereby...

>  	return;
>  
>  clean_guest_os_id:
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 36a562218010..999f5ac82fe9 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -215,7 +215,7 @@ static int hv_vtl_wakeup_secondary_cpu(int apicid, unsigned long start_eip)
>  	return hv_vtl_bringup_vcpu(vp_id, start_eip);
>  }
>  
> -static int __init hv_vtl_early_init(void)
> +int __init hv_vtl_early_init(void)
>  {
>  	/*
>  	 * `boot_cpu_has` returns the runtime feature support,
> @@ -230,4 +230,3 @@ static int __init hv_vtl_early_init(void)
>  
>  	return 0;
>  }
> -early_initcall(hv_vtl_early_init);
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 033b53f993c6..83019c3aaae9 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -340,8 +340,10 @@ static inline u64 hv_get_non_nested_register(unsigned int reg) { return 0; }
>  
>  #ifdef CONFIG_HYPERV_VTL_MODE
>  void __init hv_vtl_init_platform(void);
> +int __init hv_vtl_early_init(void);
>  #else
>  static inline void __init hv_vtl_init_platform(void) {}
> +static int __init hv_vtl_early_init(void) {}
>  #endif
>  
>  #include <asm-generic/mshyperv.h>

Acked-by: Mathias Krause <minipli@grsecurity.net>

Thanks!