Re: [PATCH v3 3/9] selftests/sgx: Handle relocations in test enclave

[Jarkko Sakkinen]

On Sat Aug 19, 2023 at 12:43 PM EEST, Jo Van Bulck wrote:
> Static-pie binaries normally include a startup routine to perform any ELF
> relocations from .rela.dyn. Since the enclave loading process is different
> and glibc is not included, do the necessary relocation for encl_op_array
> entries manually at runtime relative to the enclave base to ensure correct
> function pointers.
>
> Signed-off-by: Jo Van Bulck <jo.vanbulck@xxxxxxxxxxxxxx>
> ---
>  tools/testing/selftests/sgx/test_encl.c   | 49 ++++++++++++++++-------
>  tools/testing/selftests/sgx/test_encl.lds |  2 +
>  2 files changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c
> index c0d639729..7633fb7cb 100644
> --- a/tools/testing/selftests/sgx/test_encl.c
> +++ b/tools/testing/selftests/sgx/test_encl.c
> @@ -119,21 +119,40 @@ static void do_encl_op_nop(void *_op)
>  
>  }
>  
> +/*
> + * Symbol placed at the start of the enclave image by the linker script.
> + * Declare this extern symbol with visibility "hidden" to ensure the
> + * compiler does not access it through the GOT.
> + */
> +extern const uint8_t __attribute__((visibility("hidden"))) __encl_base;
> +static const uint64_t encl_base = (uint64_t)&__encl_base;
> +
> +typedef void (*encl_op_t)(void *);
> +const encl_op_t encl_op_array[ENCL_OP_MAX] = {
> +	do_encl_op_put_to_buf,
> +	do_encl_op_get_from_buf,
> +	do_encl_op_put_to_addr,
> +	do_encl_op_get_from_addr,
> +	do_encl_op_nop,
> +	do_encl_eaccept,
> +	do_encl_emodpe,
> +	do_encl_init_tcs_page,
> +};
> +
>  void encl_body(void *rdi,  void *rsi)
>  {
> -	const void (*encl_op_array[ENCL_OP_MAX])(void *) = {
> -		do_encl_op_put_to_buf,
> -		do_encl_op_get_from_buf,
> -		do_encl_op_put_to_addr,
> -		do_encl_op_get_from_addr,
> -		do_encl_op_nop,
> -		do_encl_eaccept,
> -		do_encl_emodpe,
> -		do_encl_init_tcs_page,
> -	};
> -
> -	struct encl_op_header *op = (struct encl_op_header *)rdi;
> -
> -	if (op->type < ENCL_OP_MAX)
> -		(*encl_op_array[op->type])(op);
> +	struct encl_op_header *header = (struct encl_op_header *)rdi;
> +	encl_op_t op;
> +
> +	if (header->type >= ENCL_OP_MAX)
> +		return;
> +
> +	/*
> +	 * "encl_base" needs to be added, as this call site *cannot be*
> +	 * made rip-relative by the compiler, or fixed up by any other
> +	 * possible means.
> +	 */
> +	op = encl_base + encl_op_array[header->type];
> +
> +	(*op)(header);
>  }
> diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds
> index 62d37160f..b86c86060 100644
> --- a/tools/testing/selftests/sgx/test_encl.lds
> +++ b/tools/testing/selftests/sgx/test_encl.lds
> @@ -32,6 +32,8 @@ SECTIONS
>  		*(.note*)
>  		*(.debug*)
>  		*(.eh_frame*)
> +		*(.dyn*)
> +		*(.gnu.hash)
>  	}
>  }
>  
> -- 
> 2.25.1

Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>

BR, Jarkko