linux-kernel - Re: [PATCH v4 02/13] selftests/sgx: Produce static-pie executable for test enclave

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]

Message-ID: <8eeec322fa4c85749c05dcbc570ac385d8ad88b6.camel@intel.com>
Date:   Tue, 29 Aug 2023 10:55:54 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
        "jarkko@...nel.org" <jarkko@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Van Bulck, Jo" <jo.vanbulck@...kuleuven.be>
CC:     "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH v4 02/13] selftests/sgx: Produce static-pie executable for
 test enclave

On Fri, 2023-08-25 at 15:32 +0200, Jo Van Bulck wrote:
> The current combination of -static and -fPIC creates a static executable
> with position-dependent addresses for global variables. Use -static-pie
> and -fPIE to create a proper static position independent executable that
> can be loaded at any address without a dynamic linker.
> 
> Link: https://lore.kernel.org/all/f9c24d89-ed72-7d9e-c650-050d722c6b04@cs.kuleuven.be/
> Signed-off-by: Jo Van Bulck <jo.vanbulck@...kuleuven.be>
> Reviewed-by: Jarkko Sakkinen <jarkko@...nel.org>
> ---
>  tools/testing/selftests/sgx/Makefile              | 2 +-
>  tools/testing/selftests/sgx/test_encl.lds         | 1 +
>  tools/testing/selftests/sgx/test_encl_bootstrap.S | 9 ++++++---
>  3 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
> index 50aab6b57da3..1d6315a2e5f5 100644
> --- a/tools/testing/selftests/sgx/Makefile
> +++ b/tools/testing/selftests/sgx/Makefile
> @@ -13,7 +13,7 @@ endif
>  
>  INCLUDES := -I$(top_srcdir)/tools/include
>  HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack
> -ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \
> +ENCL_CFLAGS := -Wall -Werror -static-pie -nostdlib -nostartfiles -fPIE \
>  	       -fno-stack-protector -mrdrnd $(INCLUDES)
>  
>  TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx
> diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds
> index a1ec64f7d91f..62d37160f59b 100644
> --- a/tools/testing/selftests/sgx/test_encl.lds
> +++ b/tools/testing/selftests/sgx/test_encl.lds
> @@ -10,6 +10,7 @@ PHDRS
>  SECTIONS
>  {
>  	. = 0;
> +        __encl_base = .;
>  	.tcs : {
>  		*(.tcs*)
>  	} : tcs
> diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S
> index 03ae0f57e29d..4e55d91566c4 100644
> --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S
> +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S
> @@ -42,9 +42,12 @@
>  encl_entry:
>  	# RBX contains the base address for TCS, which is the first address
>  	# inside the enclave for TCS #1 and one page into the enclave for
> -	# TCS #2. By adding the value of encl_stack to it, we get
> -	# the absolute address for the stack.
> -	lea	(encl_stack)(%rbx), %rax
> +	# TCS #2. First make it relative by substracting __encl_base and
> +	# then add the address of encl_stack to get the address for the stack.
> +	lea __encl_base(%rip), %rax
> +	sub %rax, %rbx
> +	lea encl_stack(%rip), %rax
> +	add %rbx, %rax

Could you please add the linker error (as you mentioned in the v3) to the
changelog to justify this code change?

And sigh.. I still don't quite understand why linker complains

	lea	(encl_stack)(%rbx), %rax

because ....

>  	jmp encl_entry_core
>  encl_dyn_entry:
>  	# Entry point for dynamically created TCS page expected to follow

we have a 

	lea -1(%rbx), %rax

right here.

Can't the compiler/linker just treat it as an immediate just like -1?  :-(

Anyway, I like this code change:

Acked-by: Kai Huang <kai.huang@...el.com>