x86/hyperv: Remove hv_vtl_early_init initcall

Message ID 1695185552-19910-1-git-send-email-ssengar@linux.microsoft.com
State New
Headers
Series x86/hyperv: Remove hv_vtl_early_init initcall |

Commit Message

Saurabh Singh Sengar Sept. 20, 2023, 4:52 a.m. UTC
  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(-)
  

Comments

Mathias Krause Sept. 21, 2023, 7:17 a.m. UTC | #1
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!
  
Saurabh Singh Sengar Sept. 21, 2023, 5:14 p.m. UTC | #2
On Thu, Sep 21, 2023 at 09:17:24AM +0200, Mathias Krause wrote:
> 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

vhv_vtl_early_init need to be replaced with hv_vtl_early_init here.

Wei,

Do you need a V2 for it ?

- Saurabh
  
Mathias Krause Sept. 21, 2023, 6:27 p.m. UTC | #3
Missed it in my review, but the kernel bot already noticed it, so....

On 20.09.23 06:52, Saurabh Sengar wrote:
> [...]
> 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) {}

static inline

>  #endif
>  
>  #include <asm-generic/mshyperv.h>

Thanks,
Mathias
  
Saurabh Singh Sengar Sept. 22, 2023, 3:45 a.m. UTC | #4
On Thu, Sep 21, 2023 at 08:27:09PM +0200, Mathias Krause wrote:
> Missed it in my review, but the kernel bot already noticed it, so....
> 
> On 20.09.23 06:52, Saurabh Sengar wrote:
> > [...]
> > 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) {}
> 
> static inline

Thanks, will send v2 fixing this and the other typo in commit

Regards,
Saurabh

> 
> >  #endif
> >  
> >  #include <asm-generic/mshyperv.h>
> 
> Thanks,
> Mathias
  
Liu, Yujie Sept. 28, 2023, 1:36 a.m. UTC | #5
Hi Saurabh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.6-rc2]
[also build test WARNING on linus/master next-20230920]
[cannot apply to tip/x86/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Saurabh-Sengar/x86-hyperv-Remove-hv_vtl_early_init-initcall/20230920-125357
base:   v6.6-rc2
patch link:    https://lore.kernel.org/r/1695185552-19910-1-git-send-email-ssengar%40linux.microsoft.com
patch subject: [PATCH] x86/hyperv: Remove hv_vtl_early_init initcall
config: i386-tinyconfig (https://download.01.org/0day-ci/archive/20230920/202309201327.J4pfXnUv-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230920/202309201327.J4pfXnUv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <yujie.liu@intel.com>
| Closes: https://lore.kernel.org/r/202309201327.J4pfXnUv-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from arch/x86/mm/pat/set_memory.c:36:
   arch/x86/include/asm/mshyperv.h: In function 'hv_vtl_early_init':
   arch/x86/include/asm/mshyperv.h:346:1: error: no return statement in function returning non-void [-Werror=return-type]
     346 | static int __init hv_vtl_early_init(void) {}
         | ^~~~~~
   arch/x86/include/asm/mshyperv.h: At top level:
>> arch/x86/include/asm/mshyperv.h:346:19: warning: 'hv_vtl_early_init' defined but not used [-Wunused-function]
     346 | static int __init hv_vtl_early_init(void) {}
         |                   ^~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/hv_vtl_early_init +346 arch/x86/include/asm/mshyperv.h

79cadff2d92bb8 Vitaly Kuznetsov 2017-08-02  339  
765e33f5211ab6 Michael Kelley   2019-05-30  340  
3be1bc2fe9d2e4 Saurabh Sengar   2023-04-10  341  #ifdef CONFIG_HYPERV_VTL_MODE
3be1bc2fe9d2e4 Saurabh Sengar   2023-04-10  342  void __init hv_vtl_init_platform(void);
9081cfc5d0e7fa Saurabh Sengar   2023-09-19  343  int __init hv_vtl_early_init(void);
3be1bc2fe9d2e4 Saurabh Sengar   2023-04-10  344  #else
3be1bc2fe9d2e4 Saurabh Sengar   2023-04-10  345  static inline void __init hv_vtl_init_platform(void) {}
9081cfc5d0e7fa Saurabh Sengar   2023-09-19 @346  static int __init hv_vtl_early_init(void) {}
3be1bc2fe9d2e4 Saurabh Sengar   2023-04-10  347  #endif
3be1bc2fe9d2e4 Saurabh Sengar   2023-04-10  348
  

Patch

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();
+
 	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>