linux-kernel - Re: [PATCHv5 03/30] x86/tdx: Add __tdx_module_call() and __tdx

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Mar 2022 00:48:28 +0300
From:   "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     tglx@...utronix.de, mingo@...hat.com, dave.hansen@...el.com,
        luto@...nel.org, peterz@...radead.org,
        sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
        ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
        hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
        joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
        pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
        tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
        thomas.lendacky@....com, brijesh.singh@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCHv5 03/30] x86/tdx: Add __tdx_module_call() and
 __tdx_hypercall() helper functions

On Fri, Mar 11, 2022 at 12:20:59AM +0300, Kirill A. Shutemov wrote:
> > > +	/*
> > > +	 * RAX==0 indicates a failure of the TDVMCALL mechanism itself and that
> > > +	 * something has gone horribly wrong with the TDX module.
> > > +	 *
> > > +	 * The return status of the hypercall operation is in a separate
> > > +	 * register (in R10). Hypercall errors are a part of normal operation
> > > +	 * and are handled by callers.
> > > +	 */
> > > +	testq %rax, %rax
> > > +	jne .Lpanic
> > 
> > Hm, can this call a C function which does the panic so that a proper
> > error message is dumped to the user so that at least she knows where the
> > panic comes from?
> 
> Sure we can. But it would look somewhat clunky.

Here how it can look like. Is it what you want?

diff --git a/arch/x86/boot/compressed/tdx.c b/arch/x86/boot/compressed/tdx.c
index f00fd3a39b64..b26eab2c3c59 100644
--- a/arch/x86/boot/compressed/tdx.c
+++ b/arch/x86/boot/compressed/tdx.c
@@ -3,6 +3,7 @@
 #include "../cpuflags.h"
 #include "../string.h"
 #include "../io.h"
+#include "error.h"

 #include <vdso/limits.h>
 #include <uapi/asm/vmx.h>
@@ -16,6 +17,11 @@ bool early_is_tdx_guest(void)
 	return tdx_guest_detected;
 }

+void __tdx_hypercall_failed(void)
+{
+	error("TDVMCALL failed. TDX module bug?");
+}
+
 static inline unsigned int tdx_io_in(int size, u16 port)
 {
 	struct tdx_hypercall_args args = {
diff --git a/arch/x86/coco/tdx/tdcall.S b/arch/x86/coco/tdx/tdcall.S
index 22832f19df2c..f39de4b01a9c 100644
--- a/arch/x86/coco/tdx/tdcall.S
+++ b/arch/x86/coco/tdx/tdcall.S
@@ -197,5 +197,5 @@ SYM_FUNC_START(__tdx_hypercall)

 	retq
 .Lpanic:
-	ud2
+	call __tdx_hypercall_failed
 SYM_FUNC_END(__tdx_hypercall)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 8e19694d33e2..29fc5941b80c 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -53,6 +53,11 @@ static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15)
 	return __tdx_hypercall(&args, 0);
 }

+void __tdx_hypercall_failed(void)
+{
+	panic("TDVMCALL failed. TDX module bug?");
+}
+
 /*
  * The TDG.VP.VMCALL-Instruction-execution sub-functions are defined
  * independently from but are currently matched 1:1 with VMX EXIT_REASONs.
-- 
 Kirill A. Shutemov