Re: [PATCH v4 03/13] selftests/sgx: Handle relocations in test enclave

From: Jo Van Bulck
Date: Thu Aug 31 2023 - 08:48:44 EST

Next message: Christoph Hellwig: "Re: [PATCH v7 9/9] swiotlb: search the software IO TLB only if the device makes use of it"
Previous message: Sumit Garg: "Re: [PATCH] lkdtm/bugs: add test for panic() with stuck secondary CPUs"
In reply to: Huang, Kai: "Re: [PATCH v4 03/13] selftests/sgx: Handle relocations in test enclave"
Next in thread: Jo Van Bulck: "[PATCH v4 10/13] selftests/sgx: Fix uninitialized pointer dereferences"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 28.08.23 15:15, Huang, Kai wrote:

I am wondering is this the right justification for _this_ particular patch?

Even above paragraph is true, the existing code w/o this patch can work because
the generated asm code uses "lea (-xxx)(%rip), %<reg>" to get the right address
and store it to the array on the stack.

Yes, I agree the current code *happens* to work with this explicit array initialization.

It stops to work because you want to use -Os, in which case the generated asm
code instead initializes the array by copying an array (which has function
addresses starting from 0) generated by the compiler/linker.

I'd say the compiler is free to perform this sensible optimization, as long as it marks any relocations in .rela.dyn. Thus, the *real* reason why it stops to work is that the enclave does not include a startup routine to perform any ELF relocations from .rela.dyn (as included in glibc).

The minimal fix, done in this patch, is to not include a full .rela.dyn relocation routine with all the overheads of parsing, but simply manually relocate the only place where this may be needed, ie the function pointer table. Ultimately, I could imagine a further enhancement may also be to parse .rela.dyn at build time and make sure no other relocations are there (outside the false positives for the TCS as discussed earlier).

So to me the true justification should be "using -Os breaks the code".

I'd say compiler optimizations should not break correct code. In other words, the main objective of this patch series is to avoid reliance on undefined, compiler-specific behavior that can make the test results unpredictable and fragile as compiler versions or options may change in the future.

Or do
you think "the compiler generating code to initialize the array on the stack
using RIP-relative addressing to get the function address" is completely a lucky
thing?

To some extent, yes. While I only saw this with -Os for gcc, I found that clang never initializes the array on the stack and this may also change for gcc at any point I'd expect.

For reference, I'm including the full encl_body assembly for both gcc and clang for -O{1,2,3,s,g} at the bottom of this email.

Anyway, it will be very helpful to include the assembly code generated both w/
and w/o using -Os here to the changelog to demonstrate the problem and we need
this patch to fixup. >
Without those information, it's basically very hard for people to understand why
this is needed. This will save maintainer's time, and make git blamer's life
easy in the future.

Makes sense, will do this for the next revision.

+ /*
+ * 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.
+ */

Is it better to explicitly call out the compiler generates RIP-relative
addressing code to get the address associated with '__encl_base' symbol, so we
can get the actual enclave base during runtime?

Maybe it's obvious, but I am not sure :-)

+ op = ((uint64_t)&__encl_base) + encl_op_array[header->type];
+
+ (*op)(header);
}

I'm including a comment on this a few lines higher, where __encl_base is declared.

Best,
Jo

------
clang.-O0.log.elf

0000000000002000 <encl_body>:
2000: 55 push %rbp
2001: 48 89 e5 mov %rsp,%rbp
2004: 48 83 ec 60 sub $0x60,%rsp
2008: 48 8d 05 f1 1f 00 00 lea 0x1ff1(%rip),%rax # 4000 <encl_entry_core+0x1b77>
200f: 48 89 7d f8 mov %rdi,-0x8(%rbp)
2013: 48 89 75 f0 mov %rsi,-0x10(%rbp)
2017: 48 8d 4d b0 lea -0x50(%rbp),%rcx
201b: 48 89 cf mov %rcx,%rdi
201e: 48 89 c6 mov %rax,%rsi
2021: ba 40 00 00 00 mov $0x40,%edx
2026: e8 95 03 00 00 call 23c0 <memcpy>
202b: 48 8b 45 f8 mov -0x8(%rbp),%rax
202f: 48 89 45 a8 mov %rax,-0x58(%rbp)
2033: 48 8b 45 a8 mov -0x58(%rbp),%rax
2037: 48 83 38 08 cmpq $0x8,(%rax)
203b: 0f 83 15 00 00 00 jae 2056 <encl_body+0x56>
2041: 48 8b 45 a8 mov -0x58(%rbp),%rax
2045: 48 8b 00 mov (%rax),%rax
2048: 48 8b 44 c5 b0 mov -0x50(%rbp,%rax,8),%rax
204d: 48 8b 4d a8 mov -0x58(%rbp),%rcx
2051: 48 89 cf mov %rcx,%rdi
2054: ff d0 call *%rax
2056: 48 83 c4 60 add $0x60,%rsp
205a: 5d pop %rbp
205b: c3 ret

------
clang.-O1.log.elf

0000000000002000 <encl_body>:
2000: 50 push %rax
2001: 48 8b 07 mov (%rdi),%rax
2004: 48 83 f8 07 cmp $0x7,%rax
2008: 77 0a ja 2014 <encl_body+0x14>
200a: 48 8d 0d ef 1f 00 00 lea 0x1fef(%rip),%rcx # 4000 <encl_entry_core+0x1d86>
2011: ff 14 c1 call *(%rcx,%rax,8)
2014: 58 pop %rax
2015: c3 ret

------
clang.-O2.log.elf

0000000000002000 <encl_body>:
2000: 48 8b 07 mov (%rdi),%rax
2003: 48 83 f8 07 cmp $0x7,%rax
2007: 77 0a ja 2013 <encl_body+0x13>
2009: 48 8d 0d f0 1f 00 00 lea 0x1ff0(%rip),%rcx # 4000 <encl_entry_core+0x1cfa>
2010: ff 24 c1 jmp *(%rcx,%rax,8)
2013: c3 ret

------
clang.-O3.log.elf

0000000000002000 <encl_body>:
2000: 48 8b 07 mov (%rdi),%rax
2003: 48 83 f8 07 cmp $0x7,%rax
2007: 77 0a ja 2013 <encl_body+0x13>
2009: 48 8d 0d f0 1f 00 00 lea 0x1ff0(%rip),%rcx # 4000 <encl_entry_core+0x1cfa>
2010: ff 24 c1 jmp *(%rcx,%rax,8)
2013: c3 ret

------
clang.-Ofast.log.elf

0000000000002000 <encl_body>:
2000: 48 8b 07 mov (%rdi),%rax
2003: 48 83 f8 07 cmp $0x7,%rax
2007: 77 0a ja 2013 <encl_body+0x13>
2009: 48 8d 0d f0 1f 00 00 lea 0x1ff0(%rip),%rcx # 4000 <encl_entry_core+0x1cfa>
2010: ff 24 c1 jmp *(%rcx,%rax,8)
2013: c3 ret

------
clang.-Og.log.elf

0000000000002000 <encl_body>:
2000: 50 push %rax
2001: 48 8b 07 mov (%rdi),%rax
2004: 48 83 f8 07 cmp $0x7,%rax
2008: 77 0a ja 2014 <encl_body+0x14>
200a: 48 8d 0d ef 1f 00 00 lea 0x1fef(%rip),%rcx # 4000 <encl_entry_core+0x1d86>
2011: ff 14 c1 call *(%rcx,%rax,8)
2014: 58 pop %rax
2015: c3 ret

------
clang.-Os.log.elf

0000000000002000 <encl_body>:
2000: 48 8b 07 mov (%rdi),%rax
2003: 48 83 f8 07 cmp $0x7,%rax
2007: 77 0a ja 2013 <encl_body+0x13>
2009: 48 8d 0d f0 1f 00 00 lea 0x1ff0(%rip),%rcx # 4000 <encl_entry_core+0x1e36>
2010: ff 24 c1 jmp *(%rcx,%rax,8)
2013: c3 ret

------
gcc.-O0.log.elf

00000000000023f4 <encl_body>:
23f4: f3 0f 1e fa endbr64
23f8: 55 push %rbp
23f9: 48 89 e5 mov %rsp,%rbp
23fc: 48 83 ec 60 sub $0x60,%rsp
2400: 48 89 7d a8 mov %rdi,-0x58(%rbp)
2404: 48 89 75 a0 mov %rsi,-0x60(%rbp)
2408: 48 8d 05 ec fe ff ff lea -0x114(%rip),%rax # 22fb <do_encl_op_put_to_buf>
240f: 48 89 45 b0 mov %rax,-0x50(%rbp)
2413: 48 8d 05 18 ff ff ff lea -0xe8(%rip),%rax # 2332 <do_encl_op_get_from_buf>
241a: 48 89 45 b8 mov %rax,-0x48(%rbp)
241e: 48 8d 05 44 ff ff ff lea -0xbc(%rip),%rax # 2369 <do_encl_op_put_to_addr>
2425: 48 89 45 c0 mov %rax,-0x40(%rbp)
2429: 48 8d 05 77 ff ff ff lea -0x89(%rip),%rax # 23a7 <do_encl_op_get_from_addr>
2430: 48 89 45 c8 mov %rax,-0x38(%rbp)
2434: 48 8d 05 aa ff ff ff lea -0x56(%rip),%rax # 23e5 <do_encl_op_nop>
243b: 48 89 45 d0 mov %rax,-0x30(%rbp)
243f: 48 8d 05 4f fc ff ff lea -0x3b1(%rip),%rax # 2095 <do_encl_eaccept>
2446: 48 89 45 d8 mov %rax,-0x28(%rbp)
244a: 48 8d 05 af fb ff ff lea -0x451(%rip),%rax # 2000 <do_encl_emodpe>
2451: 48 89 45 e0 mov %rax,-0x20(%rbp)
2455: 48 8d 05 72 fd ff ff lea -0x28e(%rip),%rax # 21ce <do_encl_init_tcs_page>
245c: 48 89 45 e8 mov %rax,-0x18(%rbp)
2460: 48 8b 45 a8 mov -0x58(%rbp),%rax
2464: 48 89 45 f8 mov %rax,-0x8(%rbp)
2468: 48 8b 45 f8 mov -0x8(%rbp),%rax
246c: 48 8b 00 mov (%rax),%rax
246f: 48 83 f8 07 cmp $0x7,%rax
2473: 77 15 ja 248a <encl_body+0x96>
2475: 48 8b 45 f8 mov -0x8(%rbp),%rax
2479: 48 8b 00 mov (%rax),%rax
247c: 48 8b 54 c5 b0 mov -0x50(%rbp,%rax,8),%rdx
2481: 48 8b 45 f8 mov -0x8(%rbp),%rax
2485: 48 89 c7 mov %rax,%rdi
2488: ff d2 call *%rdx
248a: 90 nop
248b: c9 leave
248c: c3 ret

------
gcc.-O1.log.elf

0000000000002239 <encl_body>:
2239: f3 0f 1e fa endbr64
223d: 48 83 ec 48 sub $0x48,%rsp
2241: 48 8d 05 b6 fe ff ff lea -0x14a(%rip),%rax # 20fe <do_encl_op_put_to_buf>
2248: 48 89 04 24 mov %rax,(%rsp)
224c: 48 8d 05 c5 fe ff ff lea -0x13b(%rip),%rax # 2118 <do_encl_op_get_from_buf>
2253: 48 89 44 24 08 mov %rax,0x8(%rsp)
2258: 48 8d 05 d3 fe ff ff lea -0x12d(%rip),%rax # 2132 <do_encl_op_put_to_addr>
225f: 48 89 44 24 10 mov %rax,0x10(%rsp)
2264: 48 8d 05 de fe ff ff lea -0x122(%rip),%rax # 2149 <do_encl_op_get_from_addr>
226b: 48 89 44 24 18 mov %rax,0x18(%rsp)
2270: 48 8d 05 e9 fe ff ff lea -0x117(%rip),%rax # 2160 <do_encl_op_nop>
2277: 48 89 44 24 20 mov %rax,0x20(%rsp)
227c: 48 8d 05 e9 fd ff ff lea -0x217(%rip),%rax # 206c <do_encl_eaccept>
2283: 48 89 44 24 28 mov %rax,0x28(%rsp)
2288: 48 8d 05 71 fd ff ff lea -0x28f(%rip),%rax # 2000 <do_encl_emodpe>
228f: 48 89 44 24 30 mov %rax,0x30(%rsp)
2294: 48 8d 05 ca fe ff ff lea -0x136(%rip),%rax # 2165 <do_encl_init_tcs_page>
229b: 48 89 44 24 38 mov %rax,0x38(%rsp)
22a0: 48 8b 07 mov (%rdi),%rax
22a3: 48 83 f8 07 cmp $0x7,%rax
22a7: 77 03 ja 22ac <encl_body+0x73>
22a9: ff 14 c4 call *(%rsp,%rax,8)
22ac: 48 83 c4 48 add $0x48,%rsp
22b0: c3 ret

------
gcc.-O2.log.elf

0000000000002210 <encl_body>:
2210: f3 0f 1e fa endbr64
2214: 48 8d 05 25 ff ff ff lea -0xdb(%rip),%rax # 2140 <do_encl_op_put_to_buf>
221b: 48 89 44 24 b8 mov %rax,-0x48(%rsp)
2220: 48 8d 05 49 ff ff ff lea -0xb7(%rip),%rax # 2170 <do_encl_op_get_from_buf>
2227: 48 89 44 24 c0 mov %rax,-0x40(%rsp)
222c: 48 8d 05 6d ff ff ff lea -0x93(%rip),%rax # 21a0 <do_encl_op_put_to_addr>
2233: 48 89 44 24 c8 mov %rax,-0x38(%rsp)
2238: 48 8d 05 91 ff ff ff lea -0x6f(%rip),%rax # 21d0 <do_encl_op_get_from_addr>
223f: 48 89 44 24 d0 mov %rax,-0x30(%rsp)
2244: 48 8d 05 b5 ff ff ff lea -0x4b(%rip),%rax # 2200 <do_encl_op_nop>
224b: 48 89 44 24 d8 mov %rax,-0x28(%rsp)
2250: 48 8d 05 f9 fd ff ff lea -0x207(%rip),%rax # 2050 <do_encl_eaccept>
2257: 48 89 44 24 e0 mov %rax,-0x20(%rsp)
225c: 48 8d 05 9d fd ff ff lea -0x263(%rip),%rax # 2000 <do_encl_emodpe>
2263: 48 89 44 24 e8 mov %rax,-0x18(%rsp)
2268: 48 8d 05 31 fe ff ff lea -0x1cf(%rip),%rax # 20a0 <do_encl_init_tcs_page>
226f: 48 89 44 24 f0 mov %rax,-0x10(%rsp)
2274: 48 8b 07 mov (%rdi),%rax
2277: 48 83 f8 07 cmp $0x7,%rax
227b: 77 0b ja 2288 <encl_body+0x78>
227d: ff 64 c4 b8 jmp *-0x48(%rsp,%rax,8)
2281: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
2288: c3 ret

------
gcc.-O3.log.elf

0000000000002220 <encl_body>:
2220: f3 0f 1e fa endbr64
2224: 48 8d 05 55 ff ff ff lea -0xab(%rip),%rax # 2180 <do_encl_op_get_from_buf>
222b: 48 8d 15 3e ff ff ff lea -0xc2(%rip),%rdx # 2170 <do_encl_op_put_to_buf>
2232: 66 48 0f 6e c2 movq %rdx,%xmm0
2237: 66 48 0f 6e c8 movq %rax,%xmm1
223c: 48 8d 0d 4d ff ff ff lea -0xb3(%rip),%rcx # 2190 <do_encl_op_put_to_addr>
2243: 66 0f 6c c1 punpcklqdq %xmm1,%xmm0
2247: 48 8d 05 82 ff ff ff lea -0x7e(%rip),%rax # 21d0 <do_encl_op_get_from_addr>
224e: 48 8d 35 bb ff ff ff lea -0x45(%rip),%rsi # 2210 <do_encl_op_nop>
2255: 66 48 0f 6e d0 movq %rax,%xmm2
225a: 0f 29 44 24 b8 movaps %xmm0,-0x48(%rsp)
225f: 66 48 0f 6e c1 movq %rcx,%xmm0
2264: 48 8d 05 e5 fd ff ff lea -0x21b(%rip),%rax # 2050 <do_encl_eaccept>
226b: 66 0f 6c c2 punpcklqdq %xmm2,%xmm0
226f: 66 48 0f 6e d8 movq %rax,%xmm3
2274: 48 8d 15 85 fd ff ff lea -0x27b(%rip),%rdx # 2000 <do_encl_emodpe>
227b: 0f 29 44 24 c8 movaps %xmm0,-0x38(%rsp)
2280: 66 48 0f 6e c6 movq %rsi,%xmm0
2285: 48 8d 05 14 fe ff ff lea -0x1ec(%rip),%rax # 20a0 <do_encl_init_tcs_page>
228c: 66 0f 6c c3 punpcklqdq %xmm3,%xmm0
2290: 66 48 0f 6e e0 movq %rax,%xmm4
2295: 48 8b 07 mov (%rdi),%rax
2298: 0f 29 44 24 d8 movaps %xmm0,-0x28(%rsp)
229d: 66 48 0f 6e c2 movq %rdx,%xmm0
22a2: 66 0f 6c c4 punpcklqdq %xmm4,%xmm0
22a6: 0f 29 44 24 e8 movaps %xmm0,-0x18(%rsp)
22ab: 48 83 f8 07 cmp $0x7,%rax
22af: 77 07 ja 22b8 <encl_body+0x98>
22b1: ff 64 c4 b8 jmp *-0x48(%rsp,%rax,8)
22b5: 0f 1f 00 nopl (%rax)
22b8: c3 ret

------
gcc.-Ofast.log.elf

0000000000002220 <encl_body>:
2220: f3 0f 1e fa endbr64
2224: 48 8d 05 55 ff ff ff lea -0xab(%rip),%rax # 2180 <do_encl_op_get_from_buf>
222b: 48 8d 15 3e ff ff ff lea -0xc2(%rip),%rdx # 2170 <do_encl_op_put_to_buf>
2232: 66 48 0f 6e c2 movq %rdx,%xmm0
2237: 66 48 0f 6e c8 movq %rax,%xmm1
223c: 48 8d 0d 4d ff ff ff lea -0xb3(%rip),%rcx # 2190 <do_encl_op_put_to_addr>
2243: 66 0f 6c c1 punpcklqdq %xmm1,%xmm0
2247: 48 8d 05 82 ff ff ff lea -0x7e(%rip),%rax # 21d0 <do_encl_op_get_from_addr>
224e: 48 8d 35 bb ff ff ff lea -0x45(%rip),%rsi # 2210 <do_encl_op_nop>
2255: 66 48 0f 6e d0 movq %rax,%xmm2
225a: 0f 29 44 24 b8 movaps %xmm0,-0x48(%rsp)
225f: 66 48 0f 6e c1 movq %rcx,%xmm0
2264: 48 8d 05 e5 fd ff ff lea -0x21b(%rip),%rax # 2050 <do_encl_eaccept>
226b: 66 0f 6c c2 punpcklqdq %xmm2,%xmm0
226f: 66 48 0f 6e d8 movq %rax,%xmm3
2274: 48 8d 15 85 fd ff ff lea -0x27b(%rip),%rdx # 2000 <do_encl_emodpe>
227b: 0f 29 44 24 c8 movaps %xmm0,-0x38(%rsp)
2280: 66 48 0f 6e c6 movq %rsi,%xmm0
2285: 48 8d 05 14 fe ff ff lea -0x1ec(%rip),%rax # 20a0 <do_encl_init_tcs_page>
228c: 66 0f 6c c3 punpcklqdq %xmm3,%xmm0
2290: 66 48 0f 6e e0 movq %rax,%xmm4
2295: 48 8b 07 mov (%rdi),%rax
2298: 0f 29 44 24 d8 movaps %xmm0,-0x28(%rsp)
229d: 66 48 0f 6e c2 movq %rdx,%xmm0
22a2: 66 0f 6c c4 punpcklqdq %xmm4,%xmm0
22a6: 0f 29 44 24 e8 movaps %xmm0,-0x18(%rsp)
22ab: 48 83 f8 07 cmp $0x7,%rax
22af: 77 07 ja 22b8 <encl_body+0x98>
22b1: ff 64 c4 b8 jmp *-0x48(%rsp,%rax,8)
22b5: 0f 1f 00 nopl (%rax)
22b8: c3 ret

------
gcc.-Og.log.elf

000000000000225f <encl_body>:
225f: f3 0f 1e fa endbr64
2263: 48 83 ec 48 sub $0x48,%rsp
2267: 48 8d 05 8a ff ff ff lea -0x76(%rip),%rax # 21f8 <do_encl_op_put_to_buf>
226e: 48 89 04 24 mov %rax,(%rsp)
2272: 48 8d 05 99 ff ff ff lea -0x67(%rip),%rax # 2212 <do_encl_op_get_from_buf>
2279: 48 89 44 24 08 mov %rax,0x8(%rsp)
227e: 48 8d 05 a7 ff ff ff lea -0x59(%rip),%rax # 222c <do_encl_op_put_to_addr>
2285: 48 89 44 24 10 mov %rax,0x10(%rsp)
228a: 48 8d 05 b2 ff ff ff lea -0x4e(%rip),%rax # 2243 <do_encl_op_get_from_addr>
2291: 48 89 44 24 18 mov %rax,0x18(%rsp)
2296: 48 8d 05 bd ff ff ff lea -0x43(%rip),%rax # 225a <do_encl_op_nop>
229d: 48 89 44 24 20 mov %rax,0x20(%rsp)
22a2: 48 8d 05 cc fd ff ff lea -0x234(%rip),%rax # 2075 <do_encl_eaccept>
22a9: 48 89 44 24 28 mov %rax,0x28(%rsp)
22ae: 48 8d 05 4b fd ff ff lea -0x2b5(%rip),%rax # 2000 <do_encl_emodpe>
22b5: 48 89 44 24 30 mov %rax,0x30(%rsp)
22ba: 48 8d 05 64 fe ff ff lea -0x19c(%rip),%rax # 2125 <do_encl_init_tcs_page>
22c1: 48 89 44 24 38 mov %rax,0x38(%rsp)
22c6: 48 8b 07 mov (%rdi),%rax
22c9: 48 83 f8 07 cmp $0x7,%rax
22cd: 77 03 ja 22d2 <encl_body+0x73>
22cf: ff 14 c4 call *(%rsp,%rax,8)
22d2: 48 83 c4 48 add $0x48,%rsp
22d6: c3 ret

------
gcc.-Os.log.elf

00000000000021a9 <encl_body>:
21a9: f3 0f 1e fa endbr64
21ad: 49 89 f8 mov %rdi,%r8
21b0: 48 8d 35 49 1e 00 00 lea 0x1e49(%rip),%rsi # 4000 <encl_entry_core+0x1e0f>
21b7: 48 8d 7c 24 b8 lea -0x48(%rsp),%rdi
21bc: b9 10 00 00 00 mov $0x10,%ecx
21c1: f3 a5 rep movsl %ds:(%rsi),%es:(%rdi)
21c3: 49 8b 00 mov (%r8),%rax
21c6: 48 83 f8 07 cmp $0x7,%rax
21ca: 77 0a ja 21d6 <encl_body+0x2d>
21cc: 48 8b 44 c4 b8 mov -0x48(%rsp,%rax,8),%rax
21d1: 4c 89 c7 mov %r8,%rdi
21d4: ff e0 jmp *%rax
21d6: c3 ret

Next message: Christoph Hellwig: "Re: [PATCH v7 9/9] swiotlb: search the software IO TLB only if the device makes use of it"
Previous message: Sumit Garg: "Re: [PATCH] lkdtm/bugs: add test for panic() with stuck secondary CPUs"
In reply to: Huang, Kai: "Re: [PATCH v4 03/13] selftests/sgx: Handle relocations in test enclave"
Next in thread: Jo Van Bulck: "[PATCH v4 10/13] selftests/sgx: Fix uninitialized pointer dereferences"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]