x86/hyperv/vtl: Replace real_mode_header only under Hyper-V

  Booting a CONFIG_HYPERV_VTL_MODE=y enabled kernel on bare metal or a
non-Hyper-V hypervisor leads to serve memory corruption as
hv_vtl_early_init() will run even though hv_vtl_init_platform() did not.
This skips no-oping the 'realmode_reserve' and 'realmode_init' platform
hooks, making init_real_mode() -> setup_real_mode() try to copy
'real_mode_blob' over 'real_mode_header' which we set to the stub
'hv_vtl_real_mode_header'. However, as 'real_mode_blob' isn't just a
'struct real_mode_header' -- it's the complete code! -- copying it over
'hv_vtl_real_mode_header' will corrupt quite some memory following it.

The real cause for this erroneous behaviour is that hv_vtl_early_init()
blindly assumes the kernel is running on Hyper-V, which it may not.

Fix this by making sure the code only replaces the real mode header with
the stub one iff the kernel is running under Hyper-V.

Fixes: 3be1bc2fe9d2 ("x86/hyperv: VTL support for Hyper-V")
Cc: Saurabh Sengar <ssengar@linux.microsoft.com>
Cc: stable@kernel.org
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
 arch/x86/hyperv/hv_vtl.c | 4 ++++
 1 file changed, 4 insertions(+)
  

On Mon, Sep 11, 2023 at 10:00:59AM +0200, Mathias Krause wrote:
> On 08.09.23 17:02, Saurabh Singh Sengar wrote:
> > On Fri, Sep 08, 2023 at 12:26:10PM +0200, Mathias Krause wrote:
> >> Booting a CONFIG_HYPERV_VTL_MODE=y enabled kernel on bare metal or a
> >> non-Hyper-V hypervisor leads to serve memory corruption as
> > 
> > FWIW, CONFIG_HYPERV_VTL_MODE is not expected to be enabled for non VTL
> > platforms.
> 
> Fair enough, but there's really no excuse to randomly crashing the
> kernel if one forgot to RTFM like I did. The code should (and easily
> can) handle such situations, especially if it's just a matter of a two
> line change.

Thanks, I understand your concern. We don't want people to enable this
flag by mistake and see unexpected behaviours.

To add extra safety for this flag, we can make this flag dependent on
EXPERT config.

- Saurabh
  

On 13.09.23 07:27, Saurabh Singh Sengar wrote:
> On Mon, Sep 11, 2023 at 10:00:59AM +0200, Mathias Krause wrote:
>> On 08.09.23 17:02, Saurabh Singh Sengar wrote:
>>> On Fri, Sep 08, 2023 at 12:26:10PM +0200, Mathias Krause wrote:
>>>> Booting a CONFIG_HYPERV_VTL_MODE=y enabled kernel on bare metal or a
>>>> non-Hyper-V hypervisor leads to serve memory corruption as
>>>
>>> FWIW, CONFIG_HYPERV_VTL_MODE is not expected to be enabled for non VTL
>>> platforms.
>>
>> Fair enough, but there's really no excuse to randomly crashing the
>> kernel if one forgot to RTFM like I did. The code should (and easily
>> can) handle such situations, especially if it's just a matter of a two
>> line change.
> 
> Thanks, I understand your concern. We don't want people to enable this
> flag by mistake and see unexpected behaviours.

Unexpected behaviour like randomly crashing the kernel? ;)

> To add extra safety for this flag, we can make this flag dependent on
> EXPERT config.

Well, if you want to prevent people from using it, make it depend on
BROKEN, because that's what it is. All the other hypervisor support in
the kernel (Xen, VMware, KVM, ACRN, Jailhouse, even plain Hyper-V) can
perfectly cope with getting booted on a different hypervisor or bare
metal. Why is Hyper-V's VTL mode such a special snow flake that it has
to cause random memory corruption and, in turn, crash the kernel with
spectacular (and undebugable) fireworks if it's not booted under Hyper-V?

Thanks,
Mathias
  

On Fri, Sep 15, 2023 at 09:06:15AM +0200, Mathias Krause wrote:
> On 13.09.23 07:27, Saurabh Singh Sengar wrote:
> > On Mon, Sep 11, 2023 at 10:00:59AM +0200, Mathias Krause wrote:
> >> On 08.09.23 17:02, Saurabh Singh Sengar wrote:
> >>> On Fri, Sep 08, 2023 at 12:26:10PM +0200, Mathias Krause wrote:
> >>>> Booting a CONFIG_HYPERV_VTL_MODE=y enabled kernel on bare metal or a
> >>>> non-Hyper-V hypervisor leads to serve memory corruption as
> >>>
> >>> FWIW, CONFIG_HYPERV_VTL_MODE is not expected to be enabled for non VTL
> >>> platforms.

<snip>

> 
> Well, if you want to prevent people from using it, make it depend on
> BROKEN, because that's what it is. All the other hypervisor support in
> the kernel (Xen, VMware, KVM, ACRN, Jailhouse, even plain Hyper-V) can
> perfectly cope with getting booted on a different hypervisor or bare
> metal. Why is Hyper-V's VTL mode such a special snow flake that it has
> to cause random memory corruption and, in turn, crash the kernel with
> spectacular (and undebugable) fireworks if it's not booted under Hyper-V?

'BROKEN' is certainly not the right choice here. If it is used on the
correct platform as it is designed to be nothing is broken.

The default option for CONFIG_HYPERV_VTL_MODE is set to 'N', there is
sufficient documentation for it as well. I agree there can be cases where
people can still end up enabling it, for that EXPERT is a reasonable
solution.

- Saurabh



> 
> Thanks,
> Mathias
  

On 15.09.23 13:32, Saurabh Singh Sengar wrote:
> On Fri, Sep 15, 2023 at 09:06:15AM +0200, Mathias Krause wrote:
>> On 13.09.23 07:27, Saurabh Singh Sengar wrote:
>>> On Mon, Sep 11, 2023 at 10:00:59AM +0200, Mathias Krause wrote:
>>>> On 08.09.23 17:02, Saurabh Singh Sengar wrote:
>>>>> On Fri, Sep 08, 2023 at 12:26:10PM +0200, Mathias Krause wrote:
>>>>>> Booting a CONFIG_HYPERV_VTL_MODE=y enabled kernel on bare metal or a
>>>>>> non-Hyper-V hypervisor leads to serve memory corruption as
>>>>>
>>>>> FWIW, CONFIG_HYPERV_VTL_MODE is not expected to be enabled for non VTL
>>>>> platforms.
> 
> <snip>
> 
>>
>> Well, if you want to prevent people from using it, make it depend on
>> BROKEN, because that's what it is. All the other hypervisor support in
>> the kernel (Xen, VMware, KVM, ACRN, Jailhouse, even plain Hyper-V) can
>> perfectly cope with getting booted on a different hypervisor or bare
>> metal. Why is Hyper-V's VTL mode such a special snow flake that it has
>> to cause random memory corruption and, in turn, crash the kernel with
>> spectacular (and undebugable) fireworks if it's not booted under Hyper-V?
> 
> 'BROKEN' is certainly not the right choice here. If it is used on the
> correct platform as it is designed to be nothing is broken.

This "if" is all I'm complaining about. If that assumption gets broken,
for whatever reason (a user wrongly enabling Kconfig options / a distro
trying to build a kernel that can run on many platforms / ...), the
kernel should still behave instead of corrupting memory leading to a
kernel crash -- especially if it's that dumb simple to handle this case
just fine.

So please, can you answer my question above, why VTL is such a special
snow flake that it needs no error handling nor validation of its core
assumptions like all other hypervisor code in the kernel seem to get right?

> 
> The default option for CONFIG_HYPERV_VTL_MODE is set to 'N', there is
> sufficient documentation for it as well. I agree there can be cases where
> people can still end up enabling it, for that EXPERT is a reasonable
> solution.

I don't see why this would solve anything, less so in preventing the
memory corruption angle which can easily be avoided.


Thanks,
Mathias