linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] x86/virt/tdx: Enforce no indirect calls of TDX assembly
@ 2025-06-06 13:07 Kai Huang
  2025-06-06 15:58 ` Dave Hansen
  0 siblings, 1 reply; 3+ messages in thread
From: Kai Huang @ 2025-06-06 13:07 UTC (permalink / raw)
  To: dave.hansen, peterz, tglx, bp, mingo, hpa, kirill.shutemov
  Cc: rick.p.edgecombe, x86, samitolvanen, linux-kernel

Two static inline TDX helper functions (sc_retry() and sc_retry_prerr())
pass around function pointers to assembly functions.  Normally, the
compiler inlines these helper functions, realizes that the function
pointer targets are completely static and can be resolved at compile
time, and generates direct call instructions.

But, other times (like when CONFIG_CC_OPTIMIZE_FOR_SIZE=y), the compiler
will stop inlining and instead generate indirect call instructions.

Indirect calls to assembly functions require special annotation so that
both hardware and software implementations of Control Flow Integrity
mechanisms can work correctly.  But TDX assembly functions are declared
as if they are only called directly.

Annotate both sc_retry() and sc_retry_prerr() as __always_inline so that
the compiler will always inline these helpers and generate direct call
instructions when calling TDX assembly functions (see [1]).

This was found through randconfig testing, presumably setting
CONFIG_CC_OPTIMIZE_FOR_SIZE=1 when objtool spewed a bunch of these:

vmlinux.o: warning: objtool: tdh_mem_range_block+0x7e: relocation to
!ENDBR: __seamcall_ret+0x0

Link: https://lore.kernel.org/lkml/20250605145914.GW39944@noisy.programming.kicks-ass.net/ [1]
Signed-off-by: Kai Huang <kai.huang@intel.com>
---

This is updated version (with patch subject updated) of v1 patch:

https://lore.kernel.org/lkml/20250604003848.13154-1-kai.huang@intel.com/

v1 -> v3:
 - Follow Peter's suggestion to use __always_inline, instead of
   declaring TDX assembly can be called indirectly.
 - Change patch subject accordingly.
 - Update changelog accordingly, based on Dave's version.

v2 is similar to v1 and was sent right before Peter suggested to use
__always_inline, thus feel free to ignore.

---
 arch/x86/include/asm/tdx.h  | 2 +-
 arch/x86/virt/vmx/tdx/tdx.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 8b19294600c4..7ddef3a69866 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -106,7 +106,7 @@ void tdx_init(void);
 
 typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
 
-static inline u64 sc_retry(sc_func_t func, u64 fn,
+static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
 			   struct tdx_module_args *args)
 {
 	int retry = RDRAND_RETRY_LOOPS;
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 2457d13c3f9e..c7a9a087ccaf 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -75,8 +75,9 @@ static inline void seamcall_err_ret(u64 fn, u64 err,
 			args->r9, args->r10, args->r11);
 }
 
-static inline int sc_retry_prerr(sc_func_t func, sc_err_func_t err_func,
-				 u64 fn, struct tdx_module_args *args)
+static __always_inline int sc_retry_prerr(sc_func_t func,
+					  sc_err_func_t err_func,
+					  u64 fn, struct tdx_module_args *args)
 {
 	u64 sret = sc_retry(func, fn, args);
 

base-commit: ec7714e4947909190ffb3041a03311a975350fe0
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] x86/virt/tdx: Enforce no indirect calls of TDX assembly
  2025-06-06 13:07 [PATCH v3] x86/virt/tdx: Enforce no indirect calls of TDX assembly Kai Huang
@ 2025-06-06 15:58 ` Dave Hansen
  2025-06-09 10:36   ` Huang, Kai
  0 siblings, 1 reply; 3+ messages in thread
From: Dave Hansen @ 2025-06-06 15:58 UTC (permalink / raw)
  To: Kai Huang, peterz, tglx, bp, mingo, hpa, kirill.shutemov
  Cc: rick.p.edgecombe, x86, samitolvanen, linux-kernel

It doesn't really "enforce" anything. But, oh well, I'll just fix it up
when I apply it early next week. Here's what I'll probably apply:

x86/virt/tdx: Avoid indirect calls to TDX assembly functions

Two 'static inline' TDX helper functions (sc_retry() and
sc_retry_prerr()) take function pointer arguments which refer to
assembly functions.  Normally, the compiler inlines the TDX helper,
realizes that the function pointer targets are completely static -- thus
can be resolved at compile time -- and generates direct call instructions.

But, other times (like when CONFIG_CC_OPTIMIZE_FOR_SIZE=y), the compiler
declines to inline the helpers and will instead generate indirect call
instructions.

Indirect calls to assembly functions require special annotation (for
various Control Flow Integrity mechanisms).  But TDX assembly functions
lack the special annotations and can only be called directly.

Annotate both the helpers as '__always_inline' to prod the compiler into
maintaining the direct calls. There is no guarantee here, but Peter has
volunteered to report the compiler bug if this assumption ever breaks[1].

...

> This was found through randconfig testing, presumably setting
> CONFIG_CC_OPTIMIZE_FOR_SIZE=1 when objtool spewed a bunch of these:
> 
> vmlinux.o: warning: objtool: tdh_mem_range_block+0x7e: relocation to
> !ENDBR: __seamcall_ret+0x0
> 
> Link: https://lore.kernel.org/lkml/20250605145914.GW39944@noisy.programming.kicks-ass.net/ [1]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] x86/virt/tdx: Enforce no indirect calls of TDX assembly
  2025-06-06 15:58 ` Dave Hansen
@ 2025-06-09 10:36   ` Huang, Kai
  0 siblings, 0 replies; 3+ messages in thread
From: Huang, Kai @ 2025-06-09 10:36 UTC (permalink / raw)
  To: tglx@linutronix.de, peterz@infradead.org, mingo@redhat.com,
	Hansen, Dave, kirill.shutemov@linux.intel.com, bp@alien8.de,
	hpa@zytor.com
  Cc: samitolvanen@google.com, linux-kernel@vger.kernel.org,
	Edgecombe, Rick P, x86@kernel.org

On Fri, 2025-06-06 at 08:58 -0700, Dave Hansen wrote:
> It doesn't really "enforce" anything. But, oh well, I'll just fix it up
> when I apply it early next week. Here's what I'll probably apply:

Thanks!

> 
> x86/virt/tdx: Avoid indirect calls to TDX assembly functions
> 
> Two 'static inline' TDX helper functions (sc_retry() and
> sc_retry_prerr()) take function pointer arguments which refer to
> assembly functions.  Normally, the compiler inlines the TDX helper,
> realizes that the function pointer targets are completely static -- thus
> can be resolved at compile time -- and generates direct call instructions.
> 
> But, other times (like when CONFIG_CC_OPTIMIZE_FOR_SIZE=y), the compiler
> declines to inline the helpers and will instead generate indirect call
> instructions.
> 
> Indirect calls to assembly functions require special annotation (for
> various Control Flow Integrity mechanisms).  But TDX assembly functions
> lack the special annotations and can only be called directly.
> 
> Annotate both the helpers as '__always_inline' to prod the compiler into
> maintaining the direct calls. There is no guarantee here, but Peter has
> volunteered to report the compiler bug if this assumption ever breaks[1].
> 
> ...
> 
> > This was found through randconfig testing, presumably setting
> > CONFIG_CC_OPTIMIZE_FOR_SIZE=1 when objtool spewed a bunch of these:
> > 
> > vmlinux.o: warning: objtool: tdh_mem_range_block+0x7e: relocation to
> > !ENDBR: __seamcall_ret+0x0
> > 
> > Link: https://lore.kernel.org/lkml/20250605145914.GW39944@noisy.programming.kicks-ass.net/ [1]

And sorry that I somehow missed the Fixes tag here.

Since sc_retry() and sc_retry_prerr() were introduced in two (contiguous)
commits, perhaps we need to add two Fixes tags:

Fixes: 1e66a7e27539 ("x86/virt/tdx: Handle SEAMCALL no entropy error in
common code")
Fixes: df01f5ae07dd ("x86/virt/tdx: Add SEAMCALL error printing for module
initialization")

Please let me know if you need anything more from me.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-06-09 10:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-06 13:07 [PATCH v3] x86/virt/tdx: Enforce no indirect calls of TDX assembly Kai Huang
2025-06-06 15:58 ` Dave Hansen
2025-06-09 10:36   ` Huang, Kai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).