linux-kernel - Re: [PATCH 6/8] selftests/sgx: Ensure expected enclave data buffer size and placement

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: <0885be8149082c17fc56261231c5210d693789af.camel@intel.com>
Date:   Fri, 18 Aug 2023 13:10:05 +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 6/8] selftests/sgx: Ensure expected enclave data buffer
 size and placement

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@...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
>