Message ID | 20230908102610.1039767-1-minipli@grsecurity.net |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ab0a:0:b0:3f2:4152:657d with SMTP id m10csp890984vqo; Fri, 8 Sep 2023 18:21:03 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHRrIcIMtUnX7mcLuGiM4hcOmN+/4BU5KXIfhmEMkhGJtCclhMAGjmdCfPqn2J3gZFRVr+P X-Received: by 2002:a17:902:d48c:b0:1bd:e64c:5c71 with SMTP id c12-20020a170902d48c00b001bde64c5c71mr4845376plg.21.1694222462587; Fri, 08 Sep 2023 18:21:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694222462; cv=none; d=google.com; s=arc-20160816; b=0IXvrPGl3QQ9Kyw8ZA95flMQZwt+jP6zVqjYdbnui0oqeH2ygvYt4YX2OihqvR0QWZ Q4KRIYFCwPnGAq+xEioXBZhFa/Rsx3AgSjQq5uS6ZQSlf2DqXzMa3QcyKCKG/UcosjwD 9jDCwep41Suc0qqIELKiMuEOccnoxau7qwuTRPakg/HVht2D3/FDfMfU2kr5cGm3bkEI 6m9XjSOwbgKwfowXU/Z5Nqv4JyL7FdoHWjEUqo5iqfvHRoDMfU3tjwS6ev0isOnk/Y2U vEV64ohqEP5pgaSjoWyqqSZeuGX/rGQ53umMh5SSSWFYBcZehQj4NtFUmfedU2ry14Xc p1mw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=GzirF41rnUqKpCrHpf0iIc8MJPJsNa+V/TRO9JKyJ8w=; fh=UrX/uAw0uDKkp/tIAiMc69hYc3Yvx2+xUPoN+UUAlYI=; b=PxSyG8pD9rwZBoGh6ZQV4WLRzToUh2+QxlWk04tfflWRjP8fXUF6xSeQ/nziwZPTK8 b2y+0dNqWIEBhhggSunXmZTc+TGiZPA8TB56bRCjb1EO9Re60jTkKBXOh1aRM+asIZUo Vm+4EY6ezfCrDNm8DHymu4TD7CYMqQIz5Rq4HzOwImAbB6aM5GN0rVk38xzF+4pz2JGt qVicL1ISrnSXKZBmgwywpNwHxCekYK9c8LNS+hloDiU25VJZM0Gx1OJPA20ouqGsrrF0 c+wNerPQsjGNzi624W2X//4kXrfkNzRpYScLbbR8ElXVLJSpuK/LZKpaZ8Xc9wIzVzkQ whAA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@grsecurity.net header.s=grsec header.b=jvjXqpM7; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=grsecurity.net Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id kw5-20020a170902f90500b001c320085a79si2311279plb.642.2023.09.08.18.20.45; Fri, 08 Sep 2023 18:21:02 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@grsecurity.net header.s=grsec header.b=jvjXqpM7; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=grsecurity.net Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233059AbjIHK1h (ORCPT <rfc822;lkmlmol2@gmail.com> + 99 others); Fri, 8 Sep 2023 06:27:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42762 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234322AbjIHK1g (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 8 Sep 2023 06:27:36 -0400 Received: from mail-lj1-x232.google.com (mail-lj1-x232.google.com [IPv6:2a00:1450:4864:20::232]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 346731FDB for <linux-kernel@vger.kernel.org>; Fri, 8 Sep 2023 03:27:03 -0700 (PDT) Received: by mail-lj1-x232.google.com with SMTP id 38308e7fff4ca-2bcb89b4767so32000581fa.3 for <linux-kernel@vger.kernel.org>; Fri, 08 Sep 2023 03:27:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=grsecurity.net; s=grsec; t=1694168774; x=1694773574; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=GzirF41rnUqKpCrHpf0iIc8MJPJsNa+V/TRO9JKyJ8w=; b=jvjXqpM7+1aqsRGtMdThWHuTMTyU3BLTxLVASo6Z0ZyDIqzWpamao4ee65bRkngrNH iN+8PT2m0btikOqsGnE9/iwLrEPxfwcc/WNuS4ScJJWlMPXSl5EA3y+ef+jMo7PuIKnr Q4slLWWztE1dLODASduVuC81Yosigh0rzeVQYFjRjG2rckuNwmKU8v+UuzutlAPIZNrD 90cIEnWLEwIC9wK0DFW/ldGdJ8t67W83ZvbKywNeAAAJvrLnaEJmb2kZp7dGxxL0nveZ egV46pKq1Vshf9DC4+U7h9iNOP1SDkpsuwJa2GbgCRG+/+SAhE4gzcGMwmYdlzcGMrJ6 TkXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694168774; x=1694773574; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=GzirF41rnUqKpCrHpf0iIc8MJPJsNa+V/TRO9JKyJ8w=; b=miw2D9/15RTBronLlWltbwwLkwaiFO14ZbtPkpbZb4kz8vS5qxjnsls3moxhd4G+se xC8mQ4ei78NLTXw19b7U8ptpjjSEiYafLH+GJP7xs6Bm0xhwqOFes9sZvoZq7uGV1CR6 wqDqHR9AL3gWgS9tjgbU8oOLjw5QcpuT/58qQ6pSxBFnekhkcpFltkMiwnILzmAD7Bh1 z5hY71lpE/5n2oTbQYHL3jutXeqIH+18jIsInnuUNaglBq0aYnVNjChyyLaJOudH/iTH bVZPgkN9GLHt7aV95CfSD5W499NfUybhXuCwNimn+CTx/5MPP940TfwglEcLvImVOvN7 zKbA== X-Gm-Message-State: AOJu0YxkUnPtzoDSGRMmGscI5rEne8kI0TdtJETXtH6maZLL7i7OXE4d JZk4paJNHjOgUlmfY80O3k2DZagvPm2z4l5jmCw= X-Received: by 2002:a2e:9ace:0:b0:2bd:ddf:8aa6 with SMTP id p14-20020a2e9ace000000b002bd0ddf8aa6mr1383716ljj.23.1694168773889; Fri, 08 Sep 2023 03:26:13 -0700 (PDT) Received: from bell.fritz.box (p200300f6af456100ee66353b07eac200.dip0.t-ipconnect.de. [2003:f6:af45:6100:ee66:353b:7ea:c200]) by smtp.gmail.com with ESMTPSA id q8-20020a1709064c8800b0098963eb0c3dsm844056eju.26.2023.09.08.03.26.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Sep 2023 03:26:13 -0700 (PDT) From: Mathias Krause <minipli@grsecurity.net> To: linux-hyperv@vger.kernel.org Cc: Mathias Krause <minipli@grsecurity.net>, "K. Y. Srinivasan" <kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>, linux-kernel@vger.kernel.org, Saurabh Sengar <ssengar@linux.microsoft.com>, stable@kernel.org Subject: [PATCH] x86/hyperv/vtl: Replace real_mode_header only under Hyper-V Date: Fri, 8 Sep 2023 12:26:10 +0200 Message-Id: <20230908102610.1039767-1-minipli@grsecurity.net> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1776521013074217394 X-GMAIL-MSGID: 1776521013074217394 |
Series |
x86/hyperv/vtl: Replace real_mode_header only under Hyper-V
|
|
Commit Message
Mathias Krause
Sept. 8, 2023, 10:26 a.m. UTC
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(+)
Comments
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
diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c index 57df7821d66c..54c06f4b8b4c 100644 --- a/arch/x86/hyperv/hv_vtl.c +++ b/arch/x86/hyperv/hv_vtl.c @@ -12,6 +12,7 @@ #include <asm/desc.h> #include <asm/i8259.h> #include <asm/mshyperv.h> +#include <asm/hypervisor.h> #include <asm/realmode.h> extern struct boot_params boot_params; @@ -214,6 +215,9 @@ static int hv_vtl_wakeup_secondary_cpu(int apicid, unsigned long start_eip) static int __init hv_vtl_early_init(void) { + if (!hypervisor_is_type(X86_HYPER_MS_HYPERV)) + return 0; + /* * `boot_cpu_has` returns the runtime feature support, * and here is the earliest it can be used.