Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-11-15 Thread Jarkko Sakkinen
On Wed Nov 8, 2023 at 10:46 PM EET, Dave Hansen wrote:
> On 11/8/23 12:31, Jo Van Bulck wrote:
> > Just a kind follow-up: from what I can see, this series has not been
> > merged into the x86/sgx branch of tip yet (assuming that's where it
> > should go next)?
> > 
> > Apologies if I've overlooked anything, and please let me know if there's
> > something on my end that can help!
>
> Yes, you've missed something.  For your reading pleasure:
>
> https://www.kernel.org/doc/html/latest/process/2.Process.html?highlight=merge%20window
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html
>
> I honestly didn't even think about applying this until Jarkko said
> something on 23rd.  By that point, it was far too late for it to get
> sorted out for 6.7.  So, that puts it in the next merge window.
> Specifically:
>
>   The release candidate -rc1 is the starting point for new
>   patches to be applied which are targeted for the next merge
>   window.
>
> So wait for the next -rc1, and you'll hopefully see your series get merged.

OK, great, thank you Dave.

BR, Jarkko


Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-11-15 Thread Jarkko Sakkinen
On Wed Nov 8, 2023 at 10:31 PM EET, Jo Van Bulck wrote:
> On 23.10.23 23:32, Jarkko Sakkinen wrote:
> > On Fri Oct 13, 2023 at 2:45 PM EEST, Jo Van Bulck wrote:
> >> On 10.10.23 11:44, Jarkko Sakkinen wrote:
> >>> Folks (sorry for top posting): I've now taken my old NUC7 out of the
> >>> dust and tested the series :-)
> >>>
> >>> Tested-by: Jarkko Sakkinen 
> >>
> >> Thanks for testing this Jarkko! Not sure on next steps, do you want me
> >> to re-post the series with the Tested-by tag for all commits or will you
> >> add that? Let me know if something from my side is needed.
> > 
> > Dave, can you pick these patches to the x86 tree with my tested-by
> > added? Sorry for latency. It is flu season in Finland and I've been
> > functional varying last week because of that.
>
> Just a kind follow-up: from what I can see, this series has not been 
> merged into the x86/sgx branch of tip yet (assuming that's where it 
> should go next)?
>
> Apologies if I've overlooked anything, and please let me know if there's 
> something on my end that can help!

I'm cool merging them so it is now up to Dave to pick them into the
tip tree.

> Best,
> Jo

BR, Jarkko


Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-11-09 Thread Jo Van Bulck

On 08.11.23 21:46, Dave Hansen wrote:

Yes, you've missed something.  For your reading pleasure:

https://www.kernel.org/doc/html/latest/process/2.Process.html?highlight=merge%20window
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html


My bad, thank you for pointing out the links!


I honestly didn't even think about applying this until Jarkko said
something on 23rd.  By that point, it was far too late for it to get
sorted out for 6.7.  So, that puts it in the next merge window.
Specifically:

The release candidate -rc1 is the starting point for new
patches to be applied which are targeted for the next merge
window.

So wait for the next -rc1, and you'll hopefully see your series get merged.


I see, makes sense!

Best,
Jo


Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-11-08 Thread Dave Hansen
On 11/8/23 12:31, Jo Van Bulck wrote:
> Just a kind follow-up: from what I can see, this series has not been
> merged into the x86/sgx branch of tip yet (assuming that's where it
> should go next)?
> 
> Apologies if I've overlooked anything, and please let me know if there's
> something on my end that can help!

Yes, you've missed something.  For your reading pleasure:

https://www.kernel.org/doc/html/latest/process/2.Process.html?highlight=merge%20window
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html

I honestly didn't even think about applying this until Jarkko said
something on 23rd.  By that point, it was far too late for it to get
sorted out for 6.7.  So, that puts it in the next merge window.
Specifically:

The release candidate -rc1 is the starting point for new
patches to be applied which are targeted for the next merge
window.

So wait for the next -rc1, and you'll hopefully see your series get merged.


Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-11-08 Thread Jo Van Bulck

On 23.10.23 23:32, Jarkko Sakkinen wrote:

On Fri Oct 13, 2023 at 2:45 PM EEST, Jo Van Bulck wrote:

On 10.10.23 11:44, Jarkko Sakkinen wrote:

Folks (sorry for top posting): I've now taken my old NUC7 out of the
dust and tested the series :-)

Tested-by: Jarkko Sakkinen 


Thanks for testing this Jarkko! Not sure on next steps, do you want me
to re-post the series with the Tested-by tag for all commits or will you
add that? Let me know if something from my side is needed.


Dave, can you pick these patches to the x86 tree with my tested-by
added? Sorry for latency. It is flu season in Finland and I've been
functional varying last week because of that.


Just a kind follow-up: from what I can see, this series has not been 
merged into the x86/sgx branch of tip yet (assuming that's where it 
should go next)?


Apologies if I've overlooked anything, and please let me know if there's 
something on my end that can help!


Best,
Jo


Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-10-23 Thread Jarkko Sakkinen
On Fri Oct 13, 2023 at 2:45 PM EEST, Jo Van Bulck wrote:
> On 10.10.23 11:44, Jarkko Sakkinen wrote:
> > Folks (sorry for top posting): I've now taken my old NUC7 out of the
> > dust and tested the series :-)
> > 
> > Tested-by: Jarkko Sakkinen 
>
> Thanks for testing this Jarkko! Not sure on next steps, do you want me 
> to re-post the series with the Tested-by tag for all commits or will you 
> add that? Let me know if something from my side is needed.

Dave, can you pick these patches to the x86 tree with my tested-by
added? Sorry for latency. It is flu season in Finland and I've been
functional varying last week because of that.

> > Off-topic: I wish both Intel and AMD straighten up and deliver some "home 
> > friendly" development hardware for the  latest stuff. Just my
> > stance but the biggest quality risk I see in both TDX and SNP is that
> > the patches are made by an enterprise and reviewed (properly) *only*
> > by other huge enterprises.
>
> Yes, I totally agree on this. It is really unfortunate that things like 
> SGX are not (anymore) available on home consumer hardware and you have 
> to buy expensive servers for this, which also change every new CPU 
> generation. Having some kind of "developer boards" like is more the case 
> in embedded systems would be a great and very welcome evolution, if only 
> it were to happen..
>
> > I skim status of both from time to time but yeah not much attachment
> > or motivation to do more than that as you either need a cloud access
> > or partnership with Intel or AMD. "Indie" style seems to be disliked
> > these days... You can extrapolate from this that there must be a bunch
> > of maintainers around the Linux kernel that feel the same. Not saying
> > that particularly my contribution would be that important.
> > 
> > Sort of even if let's say Intel would provide me a partner access I
> > might not be that interested because I prefer my own (physical)
> > computers.
>
> I also understand this and share the concern. FWIW for some things 
> (e.g., uarch attack research) remote access does also not really hold up 
> to bare-metal access IMO.
>
> Best,
> Jo

BR, Jarkko


Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-10-13 Thread Jo Van Bulck

On 10.10.23 14:11, Jarkko Sakkinen wrote:

Dave, since there was already sort of talk about detaching this
code from kernel tree so that Jo could work on "pure C" runtime
would it make sense to dual-license this first in the kernel tree?

E.g. Jo could send a patch once this is merged with a new SPDX
platter and we can then ack that?

Just a proposal, with the emphasis on minimal amount of work
required from each party. Also this would help with possible
(and likely) bug fixes, i.e. minimal friction on fixing the same
bug.

Later on of course, we can consider adding "libsgx-dev" as depedency
similarly as today there's a few dependencies like libelf-dev.

I'm open for alternative proposals, just throwing something that
came up mind.


Pitching in here: from my side, I'd also be fine to develop this 
bare-sgx "pure C" runtime under GPLv2 as is.


FWIW, I'd be mostly interested in and see most immediate use cases for 
such a runtime in research purposes (e.g., low-level benchmarking; rapid 
prototyping attacks/defenses; etc) and a copyleft license would be a 
good fit there IMHO.


This is not to say that I'm principally opposed to a more permissive 
(dual) license, especially if there would be a good use case for that.


But it seems to me that it may be non-trivial to build on the existing 
code base and re-license that, whereas GPLv2 would allow to fork 
immediately and also have any overlapping bug fixes seamlessly merged 
back upstream as you pointed out.


Also open to hearing alternative proposals of course!

Best,
Jo


Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-10-13 Thread Jo Van Bulck

On 10.10.23 11:44, Jarkko Sakkinen wrote:

Folks (sorry for top posting): I've now taken my old NUC7 out of the
dust and tested the series :-)

Tested-by: Jarkko Sakkinen 


Thanks for testing this Jarkko! Not sure on next steps, do you want me 
to re-post the series with the Tested-by tag for all commits or will you 
add that? Let me know if something from my side is needed.



Off-topic: I wish both Intel and AMD straighten up and deliver some
"home friendly" development hardware for the  latest stuff. Just my
stance but the biggest quality risk I see in both TDX and SNP is that
the patches are made by an enterprise and reviewed (properly) *only*
by other huge enterprises.


Yes, I totally agree on this. It is really unfortunate that things like 
SGX are not (anymore) available on home consumer hardware and you have 
to buy expensive servers for this, which also change every new CPU 
generation. Having some kind of "developer boards" like is more the case 
in embedded systems would be a great and very welcome evolution, if only 
it were to happen..



I skim status of both from time to time but yeah not much attachment
or motivation to do more than that as you either need a cloud access
or partnership with Intel or AMD. "Indie" style seems to be disliked
these days... You can extrapolate from this that there must be a bunch
of maintainers around the Linux kernel that feel the same. Not saying
that particularly my contribution would be that important.

Sort of even if let's say Intel would provide me a partner access I
might not be that interested because I prefer my own (physical)
computers.


I also understand this and share the concern. FWIW for some things 
(e.g., uarch attack research) remote access does also not really hold up 
to bare-metal access IMO.


Best,
Jo


Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-10-10 Thread Jarkko Sakkinen
Dave, since there was already sort of talk about detaching this
code from kernel tree so that Jo could work on "pure C" runtime
would it make sense to dual-license this first in the kernel tree?

E.g. Jo could send a patch once this is merged with a new SPDX
platter and we can then ack that?

Just a proposal, with the emphasis on minimal amount of work
required from each party. Also this would help with possible
(and likely) bug fixes, i.e. minimal friction on fixing the same
bug.

Later on of course, we can consider adding "libsgx-dev" as depedency
similarly as today there's a few dependencies like libelf-dev.

I'm open for alternative proposals, just throwing something that
came up mind.

BR, Jarkko

On Tue, 2023-10-10 at 12:44 +0300, Jarkko Sakkinen wrote:
> Folks (sorry for top posting): I've now taken my old NUC7 out of the
> dust and tested the series :-)
> 
> Tested-by: Jarkko Sakkinen 
> 
> Off-topic: I wish both Intel and AMD straighten up and deliver some 
> "home friendly" development hardware for the  latest stuff. Just my
> stance but the biggest quality risk I see in both TDX and SNP is that
> the patches are made by an enterprise and reviewed (properly) *only*
> by other huge enterprises.
> 
> I skim status of both from time to time but yeah not much attachment
> or motivation to do more than that as you either need a cloud access
> or partnership with Intel or AMD. "Indie" style seems to be disliked
> these days... You can extrapolate from this that there must be a bunch
> of maintainers around the Linux kernel that feel the same. Not saying
> that particularly my contribution would be that important.
> 
> Sort of even if let's say Intel would provide me a partner access I
> might not be that interested because I prefer my own (physical)
> computers.
> 
> BR, Jarkko
> 
> On Fri, 2023-10-06 at 11:51 +0200, Jo Van Bulck wrote:
> > Thank you, Kai! I'm not familiar with any next steps to get this merged 
> > upstream, but atm all commits in this series have been reviewed by at 
> > least Jarkko. Let me know if anything further is needed from my side!
> > 
> > Best,
> > Jo
> > 
> > On 05.10.23 23:25, Huang, Kai wrote:
> > > Hi Jo,
> > > 
> > > Just FYI I won't review the rest patches in this series.  One of the 
> > > reasons is
> > > I am not that familiar with the rest.  Jarkko has reviewed anyway :-).
> > > 
> > > On Thu, 2023-10-05 at 17:38 +0200, Jo Van Bulck wrote:
> > > > Hi,
> > > > 
> > > > This patch series ensures that all SGX selftests succeed when compiling 
> > > > with
> > > > optimizations (as tested with -O{0,1,2,3,s} for both gcc 11.3.0 and 
> > > > clang
> > > > 14.0.0). The aim of the patches is to avoid reliance on undefined,
> > > > compiler-specific behavior that can make the test results fragile.
> > > > 
> > > > As far as I see, all commits in this series now have an explicit 
> > > > reviewed-by
> > > > tag, so hopefully this can get merged upstream? Please let me know if 
> > > > any
> > > > concerns remain and I'd happily address them.
> > > > 
> > > > Reference output below:
> > > > 
> > > > .. Testing   gcc   -O0    [OK]
> > > > .. Testing   gcc   -O1    [OK]
> > > > .. Testing   gcc   -O2    [OK]
> > > > .. Testing   gcc   -O3    [OK]
> > > > .. Testing   gcc   -Os    [OK]
> > > > .. Testing   gcc   -Ofast [OK]
> > > > .. Testing   gcc   -Og    [OK]
> > > > .. Testing   clang -O0    [OK]
> > > > .. Testing   clang -O1    [OK]
> > > > .. Testing   clang -O2    [OK]
> > > > .. Testing   clang -O3    [OK]
> > > > .. Testing   clang -Os    [OK]
> > > > .. Testing   clang -Ofast [OK]
> > > > .. Testing   clang -Og    [OK]
> > > > 
> > > > Changelog
> > > > -
> > > > 
> > > > v7
> > > >    - Add reviewed-by tag (Jarkko)
> > > > 
> > > > v6
> > > >    - Collect final ack/reviewed-by tags (Jarkko, Kai)
> > > > 
> > > > v5
> > > >    - Reorder patches (Jarkko, Kai)
> > > >    - Include fixes tag for inline asm memory clobber patch (Kai)
> > > >    - Include linker error in static-pie commit message (Kai)
> > > >    - Include generated assembly in relocations commit (Kai)
> > > > 
> > > > v4
> > > >    - Remove redundant -nostartfiles compiler flag (Jarkko)
> > > >    - Split dynamic symbol table removal in separate commit (Kai)
> > > >    - Split redundant push/pop elimination in separate commit (Kai)
> > > >    - Remove (incomplete) register cleansing on enclave exit
> > > >    - Fix possibly uninitialized pointer dereferences in load.c
> > > > 
> > > > v3
> > > >    - Refactor encl_op_array declaration and indexing (Jarkko)
> > > >    - Annotate encl_buffer with "used" attribute (Kai)
> > > >    - Split encl_buffer size and placement commits (Kai)
> > > > 
> > > > v2
> > > >    - Add additional check for NULL pointer (Kai)
> > > >    - Refine to produce proper static-pie executable
> > > >    - Fix linker script assertions
> > > >    - Specify memory clobber for inline asm instead of volatile (Kai)
> > > >    - Clarify why encl_buffer non-static (Jarkko, Kai)
> > 

Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-10-10 Thread Jarkko Sakkinen
Folks (sorry for top posting): I've now taken my old NUC7 out of the
dust and tested the series :-)

Tested-by: Jarkko Sakkinen 

Off-topic: I wish both Intel and AMD straighten up and deliver some 
"home friendly" development hardware for the  latest stuff. Just my
stance but the biggest quality risk I see in both TDX and SNP is that
the patches are made by an enterprise and reviewed (properly) *only*
by other huge enterprises.

I skim status of both from time to time but yeah not much attachment
or motivation to do more than that as you either need a cloud access
or partnership with Intel or AMD. "Indie" style seems to be disliked
these days... You can extrapolate from this that there must be a bunch
of maintainers around the Linux kernel that feel the same. Not saying
that particularly my contribution would be that important.

Sort of even if let's say Intel would provide me a partner access I
might not be that interested because I prefer my own (physical)
computers.

BR, Jarkko

On Fri, 2023-10-06 at 11:51 +0200, Jo Van Bulck wrote:
> Thank you, Kai! I'm not familiar with any next steps to get this merged 
> upstream, but atm all commits in this series have been reviewed by at 
> least Jarkko. Let me know if anything further is needed from my side!
> 
> Best,
> Jo
> 
> On 05.10.23 23:25, Huang, Kai wrote:
> > Hi Jo,
> > 
> > Just FYI I won't review the rest patches in this series.  One of the 
> > reasons is
> > I am not that familiar with the rest.  Jarkko has reviewed anyway :-).
> > 
> > On Thu, 2023-10-05 at 17:38 +0200, Jo Van Bulck wrote:
> > > Hi,
> > > 
> > > This patch series ensures that all SGX selftests succeed when compiling 
> > > with
> > > optimizations (as tested with -O{0,1,2,3,s} for both gcc 11.3.0 and clang
> > > 14.0.0). The aim of the patches is to avoid reliance on undefined,
> > > compiler-specific behavior that can make the test results fragile.
> > > 
> > > As far as I see, all commits in this series now have an explicit 
> > > reviewed-by
> > > tag, so hopefully this can get merged upstream? Please let me know if any
> > > concerns remain and I'd happily address them.
> > > 
> > > Reference output below:
> > > 
> > > .. Testing   gcc   -O0    [OK]
> > > .. Testing   gcc   -O1    [OK]
> > > .. Testing   gcc   -O2    [OK]
> > > .. Testing   gcc   -O3    [OK]
> > > .. Testing   gcc   -Os    [OK]
> > > .. Testing   gcc   -Ofast [OK]
> > > .. Testing   gcc   -Og    [OK]
> > > .. Testing   clang -O0    [OK]
> > > .. Testing   clang -O1    [OK]
> > > .. Testing   clang -O2    [OK]
> > > .. Testing   clang -O3    [OK]
> > > .. Testing   clang -Os    [OK]
> > > .. Testing   clang -Ofast [OK]
> > > .. Testing   clang -Og    [OK]
> > > 
> > > Changelog
> > > -
> > > 
> > > v7
> > >    - Add reviewed-by tag (Jarkko)
> > > 
> > > v6
> > >    - Collect final ack/reviewed-by tags (Jarkko, Kai)
> > > 
> > > v5
> > >    - Reorder patches (Jarkko, Kai)
> > >    - Include fixes tag for inline asm memory clobber patch (Kai)
> > >    - Include linker error in static-pie commit message (Kai)
> > >    - Include generated assembly in relocations commit (Kai)
> > > 
> > > v4
> > >    - Remove redundant -nostartfiles compiler flag (Jarkko)
> > >    - Split dynamic symbol table removal in separate commit (Kai)
> > >    - Split redundant push/pop elimination in separate commit (Kai)
> > >    - Remove (incomplete) register cleansing on enclave exit
> > >    - Fix possibly uninitialized pointer dereferences in load.c
> > > 
> > > v3
> > >    - Refactor encl_op_array declaration and indexing (Jarkko)
> > >    - Annotate encl_buffer with "used" attribute (Kai)
> > >    - Split encl_buffer size and placement commits (Kai)
> > > 
> > > v2
> > >    - Add additional check for NULL pointer (Kai)
> > >    - Refine to produce proper static-pie executable
> > >    - Fix linker script assertions
> > >    - Specify memory clobber for inline asm instead of volatile (Kai)
> > >    - Clarify why encl_buffer non-static (Jarkko, Kai)
> > >    - Clarify -ffreestanding (Jarkko)
> > > 
> > > Best,
> > > Jo
> > > 
> > > Jo Van Bulck (13):
> > >    selftests/sgx: Fix uninitialized pointer dereference in error path
> > >    selftests/sgx: Fix uninitialized pointer dereferences in
> > >  encl_get_entry
> > >    selftests/sgx: Include memory clobber for inline asm in test enclave
> > >    selftests/sgx: Separate linker options
> > >    selftests/sgx: Specify freestanding environment for enclave
> > >  compilation
> > >    selftests/sgx: Remove redundant enclave base address save/restore
> > >    selftests/sgx: Produce static-pie executable for test enclave
> > >    selftests/sgx: Handle relocations in test enclave
> > >    selftests/sgx: Fix linker script asserts
> > >    selftests/sgx: Ensure test enclave buffer is entirely preserved
> > >    selftests/sgx: Ensure expected location of test enclave buffer
> > >    selftests/sgx: Discard unsupported ELF sections
> > >    selftests/sgx: Remove incomplete ABI 

Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-10-06 Thread Jo Van Bulck
Thank you, Kai! I'm not familiar with any next steps to get this merged 
upstream, but atm all commits in this series have been reviewed by at 
least Jarkko. Let me know if anything further is needed from my side!


Best,
Jo

On 05.10.23 23:25, Huang, Kai wrote:

Hi Jo,

Just FYI I won't review the rest patches in this series.  One of the reasons is
I am not that familiar with the rest.  Jarkko has reviewed anyway :-).

On Thu, 2023-10-05 at 17:38 +0200, Jo Van Bulck wrote:

Hi,

This patch series ensures that all SGX selftests succeed when compiling with
optimizations (as tested with -O{0,1,2,3,s} for both gcc 11.3.0 and clang
14.0.0). The aim of the patches is to avoid reliance on undefined,
compiler-specific behavior that can make the test results fragile.

As far as I see, all commits in this series now have an explicit reviewed-by
tag, so hopefully this can get merged upstream? Please let me know if any
concerns remain and I'd happily address them.

Reference output below:

.. Testing   gcc   -O0[OK]
.. Testing   gcc   -O1[OK]
.. Testing   gcc   -O2[OK]
.. Testing   gcc   -O3[OK]
.. Testing   gcc   -Os[OK]
.. Testing   gcc   -Ofast [OK]
.. Testing   gcc   -Og[OK]
.. Testing   clang -O0[OK]
.. Testing   clang -O1[OK]
.. Testing   clang -O2[OK]
.. Testing   clang -O3[OK]
.. Testing   clang -Os[OK]
.. Testing   clang -Ofast [OK]
.. Testing   clang -Og[OK]

Changelog
-

v7
   - Add reviewed-by tag (Jarkko)

v6
   - Collect final ack/reviewed-by tags (Jarkko, Kai)

v5
   - Reorder patches (Jarkko, Kai)
   - Include fixes tag for inline asm memory clobber patch (Kai)
   - Include linker error in static-pie commit message (Kai)
   - Include generated assembly in relocations commit (Kai)

v4
   - Remove redundant -nostartfiles compiler flag (Jarkko)
   - Split dynamic symbol table removal in separate commit (Kai)
   - Split redundant push/pop elimination in separate commit (Kai)
   - Remove (incomplete) register cleansing on enclave exit
   - Fix possibly uninitialized pointer dereferences in load.c

v3
   - Refactor encl_op_array declaration and indexing (Jarkko)
   - Annotate encl_buffer with "used" attribute (Kai)
   - Split encl_buffer size and placement commits (Kai)

v2
   - Add additional check for NULL pointer (Kai)
   - Refine to produce proper static-pie executable
   - Fix linker script assertions
   - Specify memory clobber for inline asm instead of volatile (Kai)
   - Clarify why encl_buffer non-static (Jarkko, Kai)
   - Clarify -ffreestanding (Jarkko)

Best,
Jo

Jo Van Bulck (13):
   selftests/sgx: Fix uninitialized pointer dereference in error path
   selftests/sgx: Fix uninitialized pointer dereferences in
 encl_get_entry
   selftests/sgx: Include memory clobber for inline asm in test enclave
   selftests/sgx: Separate linker options
   selftests/sgx: Specify freestanding environment for enclave
 compilation
   selftests/sgx: Remove redundant enclave base address save/restore
   selftests/sgx: Produce static-pie executable for test enclave
   selftests/sgx: Handle relocations in test enclave
   selftests/sgx: Fix linker script asserts
   selftests/sgx: Ensure test enclave buffer is entirely preserved
   selftests/sgx: Ensure expected location of test enclave buffer
   selftests/sgx: Discard unsupported ELF sections
   selftests/sgx: Remove incomplete ABI sanitization code in test enclave

  tools/testing/selftests/sgx/Makefile  | 12 ++--
  tools/testing/selftests/sgx/defines.h |  2 +
  tools/testing/selftests/sgx/load.c|  9 ++-
  tools/testing/selftests/sgx/sigstruct.c   |  5 +-
  tools/testing/selftests/sgx/test_encl.c   | 67 +--
  tools/testing/selftests/sgx/test_encl.lds | 10 +--
  .../selftests/sgx/test_encl_bootstrap.S   | 28 +++-
  7 files changed, 77 insertions(+), 56 deletions(-)





Re: [PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-10-05 Thread Huang, Kai
Hi Jo,

Just FYI I won't review the rest patches in this series.  One of the reasons is
I am not that familiar with the rest.  Jarkko has reviewed anyway :-).

On Thu, 2023-10-05 at 17:38 +0200, Jo Van Bulck wrote:
> Hi,
> 
> This patch series ensures that all SGX selftests succeed when compiling with
> optimizations (as tested with -O{0,1,2,3,s} for both gcc 11.3.0 and clang
> 14.0.0). The aim of the patches is to avoid reliance on undefined,
> compiler-specific behavior that can make the test results fragile.
> 
> As far as I see, all commits in this series now have an explicit reviewed-by
> tag, so hopefully this can get merged upstream? Please let me know if any
> concerns remain and I'd happily address them.
> 
> Reference output below:
> 
> .. Testing   gcc   -O0[OK]
> .. Testing   gcc   -O1[OK]
> .. Testing   gcc   -O2[OK]
> .. Testing   gcc   -O3[OK]
> .. Testing   gcc   -Os[OK]
> .. Testing   gcc   -Ofast [OK]
> .. Testing   gcc   -Og[OK]
> .. Testing   clang -O0[OK]
> .. Testing   clang -O1[OK]
> .. Testing   clang -O2[OK]
> .. Testing   clang -O3[OK]
> .. Testing   clang -Os[OK]
> .. Testing   clang -Ofast [OK]
> .. Testing   clang -Og[OK]
> 
> Changelog
> -
> 
> v7
>   - Add reviewed-by tag (Jarkko)
> 
> v6
>   - Collect final ack/reviewed-by tags (Jarkko, Kai)
> 
> v5
>   - Reorder patches (Jarkko, Kai)
>   - Include fixes tag for inline asm memory clobber patch (Kai)
>   - Include linker error in static-pie commit message (Kai)
>   - Include generated assembly in relocations commit (Kai)
> 
> v4
>   - Remove redundant -nostartfiles compiler flag (Jarkko)
>   - Split dynamic symbol table removal in separate commit (Kai)
>   - Split redundant push/pop elimination in separate commit (Kai)
>   - Remove (incomplete) register cleansing on enclave exit
>   - Fix possibly uninitialized pointer dereferences in load.c
> 
> v3
>   - Refactor encl_op_array declaration and indexing (Jarkko)
>   - Annotate encl_buffer with "used" attribute (Kai)
>   - Split encl_buffer size and placement commits (Kai)
> 
> v2
>   - Add additional check for NULL pointer (Kai)
>   - Refine to produce proper static-pie executable
>   - Fix linker script assertions
>   - Specify memory clobber for inline asm instead of volatile (Kai)
>   - Clarify why encl_buffer non-static (Jarkko, Kai)
>   - Clarify -ffreestanding (Jarkko)
> 
> Best,
> Jo
> 
> Jo Van Bulck (13):
>   selftests/sgx: Fix uninitialized pointer dereference in error path
>   selftests/sgx: Fix uninitialized pointer dereferences in
> encl_get_entry
>   selftests/sgx: Include memory clobber for inline asm in test enclave
>   selftests/sgx: Separate linker options
>   selftests/sgx: Specify freestanding environment for enclave
> compilation
>   selftests/sgx: Remove redundant enclave base address save/restore
>   selftests/sgx: Produce static-pie executable for test enclave
>   selftests/sgx: Handle relocations in test enclave
>   selftests/sgx: Fix linker script asserts
>   selftests/sgx: Ensure test enclave buffer is entirely preserved
>   selftests/sgx: Ensure expected location of test enclave buffer
>   selftests/sgx: Discard unsupported ELF sections
>   selftests/sgx: Remove incomplete ABI sanitization code in test enclave
> 
>  tools/testing/selftests/sgx/Makefile  | 12 ++--
>  tools/testing/selftests/sgx/defines.h |  2 +
>  tools/testing/selftests/sgx/load.c|  9 ++-
>  tools/testing/selftests/sgx/sigstruct.c   |  5 +-
>  tools/testing/selftests/sgx/test_encl.c   | 67 +--
>  tools/testing/selftests/sgx/test_encl.lds | 10 +--
>  .../selftests/sgx/test_encl_bootstrap.S   | 28 +++-
>  7 files changed, 77 insertions(+), 56 deletions(-)
> 



[PATCH v7 00/13] selftests/sgx: Fix compilation errors

2023-10-05 Thread Jo Van Bulck
Hi,

This patch series ensures that all SGX selftests succeed when compiling with
optimizations (as tested with -O{0,1,2,3,s} for both gcc 11.3.0 and clang
14.0.0). The aim of the patches is to avoid reliance on undefined,
compiler-specific behavior that can make the test results fragile.

As far as I see, all commits in this series now have an explicit reviewed-by
tag, so hopefully this can get merged upstream? Please let me know if any
concerns remain and I'd happily address them.

Reference output below:

.. Testing   gcc   -O0[OK]
.. Testing   gcc   -O1[OK]
.. Testing   gcc   -O2[OK]
.. Testing   gcc   -O3[OK]
.. Testing   gcc   -Os[OK]
.. Testing   gcc   -Ofast [OK]
.. Testing   gcc   -Og[OK]
.. Testing   clang -O0[OK]
.. Testing   clang -O1[OK]
.. Testing   clang -O2[OK]
.. Testing   clang -O3[OK]
.. Testing   clang -Os[OK]
.. Testing   clang -Ofast [OK]
.. Testing   clang -Og[OK]

Changelog
-

v7
  - Add reviewed-by tag (Jarkko)

v6
  - Collect final ack/reviewed-by tags (Jarkko, Kai)

v5
  - Reorder patches (Jarkko, Kai)
  - Include fixes tag for inline asm memory clobber patch (Kai)
  - Include linker error in static-pie commit message (Kai)
  - Include generated assembly in relocations commit (Kai)

v4
  - Remove redundant -nostartfiles compiler flag (Jarkko)
  - Split dynamic symbol table removal in separate commit (Kai)
  - Split redundant push/pop elimination in separate commit (Kai)
  - Remove (incomplete) register cleansing on enclave exit
  - Fix possibly uninitialized pointer dereferences in load.c

v3
  - Refactor encl_op_array declaration and indexing (Jarkko)
  - Annotate encl_buffer with "used" attribute (Kai)
  - Split encl_buffer size and placement commits (Kai)

v2
  - Add additional check for NULL pointer (Kai)
  - Refine to produce proper static-pie executable
  - Fix linker script assertions
  - Specify memory clobber for inline asm instead of volatile (Kai)
  - Clarify why encl_buffer non-static (Jarkko, Kai)
  - Clarify -ffreestanding (Jarkko)

Best,
Jo

Jo Van Bulck (13):
  selftests/sgx: Fix uninitialized pointer dereference in error path
  selftests/sgx: Fix uninitialized pointer dereferences in
encl_get_entry
  selftests/sgx: Include memory clobber for inline asm in test enclave
  selftests/sgx: Separate linker options
  selftests/sgx: Specify freestanding environment for enclave
compilation
  selftests/sgx: Remove redundant enclave base address save/restore
  selftests/sgx: Produce static-pie executable for test enclave
  selftests/sgx: Handle relocations in test enclave
  selftests/sgx: Fix linker script asserts
  selftests/sgx: Ensure test enclave buffer is entirely preserved
  selftests/sgx: Ensure expected location of test enclave buffer
  selftests/sgx: Discard unsupported ELF sections
  selftests/sgx: Remove incomplete ABI sanitization code in test enclave

 tools/testing/selftests/sgx/Makefile  | 12 ++--
 tools/testing/selftests/sgx/defines.h |  2 +
 tools/testing/selftests/sgx/load.c|  9 ++-
 tools/testing/selftests/sgx/sigstruct.c   |  5 +-
 tools/testing/selftests/sgx/test_encl.c   | 67 +--
 tools/testing/selftests/sgx/test_encl.lds | 10 +--
 .../selftests/sgx/test_encl_bootstrap.S   | 28 +++-
 7 files changed, 77 insertions(+), 56 deletions(-)

-- 
2.25.1