| Message ID | 20230808193145.8860-7-jo.vanbulck@cs.kuleuven.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | selftests/sgx: Fix compilation errors | expand |
On Tue, 2023-08-08 at 12:31 -0700, Jo Van Bulck wrote: > Ensure the compiler respects the size and placement of encl_buffer as > expected by the external tests manipulating page permissions: > > 1. Declare encl_buffer as global, in order to ensure that it is not > optimized away by the compiler, even when not used entirely by the test > enclave code. > > 2. Place encl_buffer in a separate section that is explicitly placed > at the start of the .data segment in the linker script to avoid the > compiler placing it somewhere else in .data. Firstly, these two problems are independent. Could you split this into two patches? One to preserve the entire buffer, the other to always place the buffer at the beginning. Secondly, as replied to v1, I think we can use "used" gcc attribute to always preserve the buffer? > > Link: https://lore.kernel.org/all/a2732938-f3db-a0af-3d68-a18060f66e79@cs.kuleuven.be/ > Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> > --- > tools/testing/selftests/sgx/test_encl.c | 9 +++++---- > tools/testing/selftests/sgx/test_encl.lds | 1 + > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c > index 5b758eaf808c..02a9e8c55e82 100644 > --- a/tools/testing/selftests/sgx/test_encl.c > +++ b/tools/testing/selftests/sgx/test_encl.c > @@ -5,11 +5,12 @@ > #include "defines.h" > > /* > - * Data buffer spanning two pages that will be placed first in .data > - * segment. Even if not used internally the second page is needed by > - * external test manipulating page permissions. > + * Data buffer spanning two pages that will be placed first in the .data > + * segment via the linker script. Even if not used internally the second page > + * is needed by external test manipulating page permissions, so do not declare > + * encl_buffer as static to make sure it is entirely preserved by the compiler. > */ > -static uint8_t encl_buffer[8192] = { 1 }; > +uint8_t __attribute__((section(".data.encl_buffer"))) encl_buffer[8192]; > > enum sgx_enclu_function { > EACCEPT = 0x5, > diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds > index 27c2527ecbc4..2ec29340ba94 100644 > --- a/tools/testing/selftests/sgx/test_encl.lds > +++ b/tools/testing/selftests/sgx/test_encl.lds > @@ -24,6 +24,7 @@ SECTIONS > } : text > > .data : { > + *(.data.encl_buffer) > *(.data*) > } : data >
On 18.08.23 06:10, Huang, Kai wrote: > Firstly, these two problems are independent. Could you split this into two > patches? One to preserve the entire buffer, the other to always place the > buffer at the beginning. Makes sense, doing this for the next revision. > Secondly, as replied to v1, I think we can use "used" gcc attribute to always > preserve the buffer? Thanks for the suggestion. Yes, I was not aware of this attribute, but it is indeed exactly what we need in this case. Applying this in the next revision. Best, Jo
diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index 5b758eaf808c..02a9e8c55e82 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -5,11 +5,12 @@ #include "defines.h" /* - * Data buffer spanning two pages that will be placed first in .data - * segment. Even if not used internally the second page is needed by - * external test manipulating page permissions. + * Data buffer spanning two pages that will be placed first in the .data + * segment via the linker script. Even if not used internally the second page + * is needed by external test manipulating page permissions, so do not declare + * encl_buffer as static to make sure it is entirely preserved by the compiler. */ -static uint8_t encl_buffer[8192] = { 1 }; +uint8_t __attribute__((section(".data.encl_buffer"))) encl_buffer[8192]; enum sgx_enclu_function { EACCEPT = 0x5, diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds index 27c2527ecbc4..2ec29340ba94 100644 --- a/tools/testing/selftests/sgx/test_encl.lds +++ b/tools/testing/selftests/sgx/test_encl.lds @@ -24,6 +24,7 @@ SECTIONS } : text .data : { + *(.data.encl_buffer) *(.data*) } : data
Ensure the compiler respects the size and placement of encl_buffer as expected by the external tests manipulating page permissions: 1. Declare encl_buffer as global, in order to ensure that it is not optimized away by the compiler, even when not used entirely by the test enclave code. 2. Place encl_buffer in a separate section that is explicitly placed at the start of the .data segment in the linker script to avoid the compiler placing it somewhere else in .data. Link: https://lore.kernel.org/all/a2732938-f3db-a0af-3d68-a18060f66e79@cs.kuleuven.be/ Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> --- tools/testing/selftests/sgx/test_encl.c | 9 +++++---- tools/testing/selftests/sgx/test_encl.lds | 1 + 2 files changed, 6 insertions(+), 4 deletions(-)