| 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
| ||
|
Message-ID: <20230906224541.2778523-1-acdunlap@google.com>
Date: Wed, 6 Sep 2023 15:45:41 -0700
From: Adam Dunlap <acdunlap@...gle.com>
To: linux-kernel@...r.kernel.org, x86@...nel.org
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>,
Tom Lendacky <thomas.lendacky@....com>,
David Hildenbrand <david@...hat.com>,
Mike Rapoport <rppt@...nel.org>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Nikunj A Dadhania <nikunj@....com>,
Adam Dunlap <acdunlap@...gle.com>,
Dionna Glaze <dionnaglaze@...gle.com>,
"Peter Zijlstra (Intel)" <peterz@...radead.org>,
Joerg Roedel <jroedel@...e.de>, Jacob Xu <jacobhxu@...gle.com>
Subject: [PATCH] x86/sev-es: Do not use copy_from_kernel_nofault in early #VC handler
In the early #VC handler before start_kernel,
boot_cpu_data.x86_virt_bits is not yet initialized.
copy_from_kernel_nofault references this variable, so it cannot be
called. Instead, simply use memcpy.
Usage of this uninitialized variable is currently causing boot failures
in the latest version of ubuntu2204 in the gcp project, but in general
reading the uninitialized variable is undefined behavior. If the
variable happens to be 0, UB also happens due to a bit shift by 64.
Fixes: 1aa9aa8ee517 ("x86/sev-es: Setup GHCB-based boot #VC handler")
Tested-by: Jacob Xu <jacobhxu@...gle.com>
Signed-off-by: Adam Dunlap <acdunlap@...gle.com>
---
arch/x86/boot/compressed/sev.c | 4 ++--
arch/x86/kernel/sev-shared.c | 4 ++--
arch/x86/kernel/sev.c | 22 ++++++++++++++--------
3 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 09dc8c187b3c..0829ae00a885 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -73,7 +73,7 @@ static inline void sev_es_wr_ghcb_msr(u64 val)
boot_wrmsr(MSR_AMD64_SEV_ES_GHCB, &m);
}
-static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt)
+static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt, bool is_early)
{
char buffer[MAX_INSN_SIZE];
int ret;
@@ -290,7 +290,7 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_GEN_REQ);
vc_ghcb_invalidate(boot_ghcb);
- result = vc_init_em_ctxt(&ctxt, regs, exit_code);
+ result = vc_init_em_ctxt(&ctxt, regs, exit_code, true);
if (result != ES_OK)
goto finish;
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 2eabccde94fb..616be2e9a663 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -176,7 +176,7 @@ static bool vc_decoding_needed(unsigned long exit_code)
static enum es_result vc_init_em_ctxt(struct es_em_ctxt *ctxt,
struct pt_regs *regs,
- unsigned long exit_code)
+ unsigned long exit_code, bool is_early)
{
enum es_result ret = ES_OK;
@@ -184,7 +184,7 @@ static enum es_result vc_init_em_ctxt(struct es_em_ctxt *ctxt,
ctxt->regs = regs;
if (vc_decoding_needed(exit_code))
- ret = vc_decode_insn(ctxt);
+ ret = vc_decode_insn(ctxt, is_early);
return ret;
}
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 1ee7bed453de..93f117e5cddf 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -270,9 +270,15 @@ static __always_inline void sev_es_wr_ghcb_msr(u64 val)
}
static int vc_fetch_insn_kernel(struct es_em_ctxt *ctxt,
- unsigned char *buffer)
+ unsigned char *buffer, bool is_early)
{
- return copy_from_kernel_nofault(buffer, (unsigned char *)ctxt->regs->ip, MAX_INSN_SIZE);
+ if (is_early) {
+ memcpy(buffer, (unsigned char *)ctxt->regs->ip, MAX_INSN_SIZE);
+ return 0;
+ } else {
+ return copy_from_kernel_nofault(buffer, (unsigned char *)ctxt->regs->ip,
+ MAX_INSN_SIZE);
+ }
}
static enum es_result __vc_decode_user_insn(struct es_em_ctxt *ctxt)
@@ -304,12 +310,12 @@ static enum es_result __vc_decode_user_insn(struct es_em_ctxt *ctxt)
return ES_DECODE_FAILED;
}
-static enum es_result __vc_decode_kern_insn(struct es_em_ctxt *ctxt)
+static enum es_result __vc_decode_kern_insn(struct es_em_ctxt *ctxt, bool is_early)
{
char buffer[MAX_INSN_SIZE];
int res, ret;
- res = vc_fetch_insn_kernel(ctxt, buffer);
+ res = vc_fetch_insn_kernel(ctxt, buffer, is_early);
if (res) {
ctxt->fi.vector = X86_TRAP_PF;
ctxt->fi.error_code = X86_PF_INSTR;
@@ -324,12 +330,12 @@ static enum es_result __vc_decode_kern_insn(struct es_em_ctxt *ctxt)
return ES_OK;
}
-static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt)
+static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt, bool is_early)
{
if (user_mode(ctxt->regs))
return __vc_decode_user_insn(ctxt);
else
- return __vc_decode_kern_insn(ctxt);
+ return __vc_decode_kern_insn(ctxt, is_early);
}
static enum es_result vc_write_mem(struct es_em_ctxt *ctxt,
@@ -1829,7 +1835,7 @@ static bool vc_raw_handle_exception(struct pt_regs *regs, unsigned long error_co
ghcb = __sev_get_ghcb(&state);
vc_ghcb_invalidate(ghcb);
- result = vc_init_em_ctxt(&ctxt, regs, error_code);
+ result = vc_init_em_ctxt(&ctxt, regs, error_code, false);
if (result == ES_OK)
result = vc_handle_exitcode(&ctxt, ghcb, error_code);
@@ -1969,7 +1975,7 @@ bool __init handle_vc_boot_ghcb(struct pt_regs *regs)
vc_ghcb_invalidate(boot_ghcb);
- result = vc_init_em_ctxt(&ctxt, regs, exit_code);
+ result = vc_init_em_ctxt(&ctxt, regs, exit_code, true);
if (result == ES_OK)
result = vc_handle_exitcode(&ctxt, boot_ghcb, exit_code);
--
2.42.0.283.g2d96d420d3-goog
Powered by blists - more mailing lists