| Message ID | 20230831134144.22686-9-jo.vanbulck@cs.kuleuven.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | selftests/sgx: Fix compilation errors | expand |
On Thu, 2023-08-31 at 15:41 +0200, 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. > > When keeping encl_op_array as a local variable on the stack, gcc without > optimizations generates code that explicitly gets the right function > addresses and stores them to create the array on the stack: > > encl_body: > /* snipped */ > lea do_encl_op_put_to_buf(%rip), %rax > mov %rax, -0x50(%rbp) > lea do_encl_op_get_from_buf(%rip), %rax > mov %rax,-0x48(%rbp) > lea do_encl_op_put_to_addr(%rip), %rax > /* snipped */ > > However, gcc -Os or clang generate more efficient code that initializes > encl_op_array by copying a "prepared copy" containing the absolute > addresses of the functions (i.e., relative to the image base starting from > 0) generated by the compiler/linker: > > encl_body: > /* snipped */ > lea prepared_copy(%rip), %rsi > lea -0x48(%rsp), %rdi > mov $0x10,%ecx > rep movsl %ds:(%rsi),%es:(%rdi) > /* snipped */ > > When building the enclave with -static-pie, the compiler/linker includes > relocation entries for the function symbols in the "prepared copy": > > Relocation section '.rela.dyn' at offset 0x4000 contains 12 entries: > Offset Info Type Symbol > /* snipped; "prepared_copy" starts at 0x6000 */ > 000000006000 000000000008 R_X86_64_RELATIVE <do_encl_emodpe> > 000000006008 000000000008 R_X86_64_RELATIVE <do_encl_eaccept> > 000000006010 000000000008 R_X86_64_RELATIVE <do_encl_op_put_to_buf> > 000000006018 000000000008 R_X86_64_RELATIVE <do_encl_op_get_from_buf> > 000000006020 000000000008 R_X86_64_RELATIVE <do_encl_op_put_to_addr> > 000000006028 000000000008 R_X86_64_RELATIVE <do_encl_op_get_from_addr> > 000000006030 000000000008 R_X86_64_RELATIVE <do_encl_op_nop> > 000000006038 000000000008 R_X86_64_RELATIVE <do_encl_init_tcs_page> > > Static-pie binaries normally include a glibc "_dl_relocate_static_pie" > routine that will perform these relocations as part of the startup. > However, since the enclave loading process is different and glibc is not > included, we cannot rely on these relocations to be performed. Without > relocations, the code would erroneously jump to the _absolute_ function > address loaded from the local copy. > > Thus, declare "encl_op_array" as global and manually relocate the loaded > function-pointer entries relative to the enclave base at runtime. This > generates the following code: > > encl_body: > /* snipped */ > lea encl_op_array(%rip), %rcx > lea __encl_base(%rip), %rax > add (%rcx,%rdx,8),%rax > jmp *%rax call *%rax ? > ret > > Link: https://lore.kernel.org/all/150d8ca8-2c66-60d1-f9fc-8e6279824e94@cs.kuleuven.be/ > Link: https://lore.kernel.org/all/5c22de5a-4b3b-1f38-9771-409b4ec7f96d@cs.kuleuven.be/#r > Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> Acked-by: Kai Huang <kai.huang@intel.com>
On 01.09.23 01:13, Huang, Kai wrote: >> encl_body: >> /* snipped */ >> lea encl_op_array(%rip), %rcx >> lea __encl_base(%rip), %rax >> add (%rcx,%rdx,8),%rax >> jmp *%rax > > call *%rax > > ? > >> ret Good catch, but this is indeed the code as generated with gcc -Os (for readability). It seems the compiler choose a JMP over a CALL (as the stack is untouched and the callee can immediately return to the caller). Somehow, the compiler still emits a RET after the JMP (which is not supposed to return here) though. I agree this is unnecessarily confusing and can simply remove the RET from the commit message. Best, Jo
On Fri, 2023-09-01 at 01:26 +0200, Jo Van Bulck wrote: > On 01.09.23 01:13, Huang, Kai wrote: > > > encl_body: > > > /* snipped */ > > > lea encl_op_array(%rip), %rcx > > > lea __encl_base(%rip), %rax > > > add (%rcx,%rdx,8),%rax > > > jmp *%rax > > > > call *%rax > > > > ? > > > > > ret > > Good catch, but this is indeed the code as generated with gcc -Os (for > readability). It seems the compiler choose a JMP over a CALL (as the > stack is untouched and the callee can immediately return to the caller). Ah, OK. I thought it's a typo, but obviously the compiler is smarter. :-) > > Somehow, the compiler still emits a RET after the JMP (which is not > supposed to return here) though. I agree this is unnecessarily confusing > and can simply remove the RET from the commit message. The RET is for encl_body itself I suppose. > > Best, > Jo
diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index ae791df3e5a5..649604c526e7 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -121,21 +121,41 @@ 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 and generates position-independent + * addressing as __encl_base(%rip), so we can get the actual enclave base + * during runtime. + */ +extern const uint8_t __attribute__((visibility("hidden"))) __encl_base; + +typedef void (*encl_op_t)(void *); +static 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; + + /* + * The enclave base address 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 = ((uint64_t)&__encl_base) + encl_op_array[header->type]; + + (*op)(header); }
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. When keeping encl_op_array as a local variable on the stack, gcc without optimizations generates code that explicitly gets the right function addresses and stores them to create the array on the stack: encl_body: /* snipped */ lea do_encl_op_put_to_buf(%rip), %rax mov %rax, -0x50(%rbp) lea do_encl_op_get_from_buf(%rip), %rax mov %rax,-0x48(%rbp) lea do_encl_op_put_to_addr(%rip), %rax /* snipped */ However, gcc -Os or clang generate more efficient code that initializes encl_op_array by copying a "prepared copy" containing the absolute addresses of the functions (i.e., relative to the image base starting from 0) generated by the compiler/linker: encl_body: /* snipped */ lea prepared_copy(%rip), %rsi lea -0x48(%rsp), %rdi mov $0x10,%ecx rep movsl %ds:(%rsi),%es:(%rdi) /* snipped */ When building the enclave with -static-pie, the compiler/linker includes relocation entries for the function symbols in the "prepared copy": Relocation section '.rela.dyn' at offset 0x4000 contains 12 entries: Offset Info Type Symbol /* snipped; "prepared_copy" starts at 0x6000 */ 000000006000 000000000008 R_X86_64_RELATIVE <do_encl_emodpe> 000000006008 000000000008 R_X86_64_RELATIVE <do_encl_eaccept> 000000006010 000000000008 R_X86_64_RELATIVE <do_encl_op_put_to_buf> 000000006018 000000000008 R_X86_64_RELATIVE <do_encl_op_get_from_buf> 000000006020 000000000008 R_X86_64_RELATIVE <do_encl_op_put_to_addr> 000000006028 000000000008 R_X86_64_RELATIVE <do_encl_op_get_from_addr> 000000006030 000000000008 R_X86_64_RELATIVE <do_encl_op_nop> 000000006038 000000000008 R_X86_64_RELATIVE <do_encl_init_tcs_page> Static-pie binaries normally include a glibc "_dl_relocate_static_pie" routine that will perform these relocations as part of the startup. However, since the enclave loading process is different and glibc is not included, we cannot rely on these relocations to be performed. Without relocations, the code would erroneously jump to the _absolute_ function address loaded from the local copy. Thus, declare "encl_op_array" as global and manually relocate the loaded function-pointer entries relative to the enclave base at runtime. This generates the following code: encl_body: /* snipped */ lea encl_op_array(%rip), %rcx lea __encl_base(%rip), %rax add (%rcx,%rdx,8),%rax jmp *%rax ret Link: https://lore.kernel.org/all/150d8ca8-2c66-60d1-f9fc-8e6279824e94@cs.kuleuven.be/ Link: https://lore.kernel.org/all/5c22de5a-4b3b-1f38-9771-409b4ec7f96d@cs.kuleuven.be/#r Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> --- tools/testing/selftests/sgx/test_encl.c | 50 +++++++++++++++++-------- 1 file changed, 35 insertions(+), 15 deletions(-)