Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make aep_trampoline.S relocatable #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Danacus
Copy link
Contributor

@Danacus Danacus commented Feb 29, 2024

Upstreaming some changes in my personal fork. If I recall correctly, this change to aep_trampoline.S was necessary to make libsgxstep relocatable. Without these changes, it is not possible to link libsgxstep in a shared object.

@jovanbulck
Copy link
Owner

thanks for opening this PR! Not sure in which exact scenario we need this? Is this related to the Rust bindings (#78 )?

Also, don't we have to do this as well for other .S files like irq_entry.S?

@Danacus
Copy link
Contributor Author

Danacus commented Mar 14, 2024

I don't think it is related to #78, but it's something I had to do when trying to build libsgxstep with -fPIC. If I recall correctly, I needed this because I was then linking libsgxstep into a shared object, which requires -fPIC.

I'm not sure if we should do this in other .S files as well.

@jovanbulck
Copy link
Owner

I see, is there a reason to compile libsgxstep with -fPIC though?

@Danacus
Copy link
Contributor Author

Danacus commented Mar 14, 2024

Perhaps my use case is a bit niche, but I was using a program that loads a .so at runtime depending on some command line argument, and that .so then loads a certain enclave and uses some libsgxstep features. In order to link code into a shared object, it needs to be compiled with -fPIC.

@jovanbulck
Copy link
Owner

makes sense, thanks for clarifying.

In fact, I've already longer wanted to enable LD_PRELOADing libsgxstep into a standard application (cf #28 ) to dynamically hook into enclave entries.

So putting this on the list to figure out and upstream!

@Danacus
Copy link
Contributor Author

Danacus commented Mar 21, 2024

Also, don't we have to do this as well for other .S files like irq_entry.S?

It turns out that we do. I was not running into this before, because I did not include these files when linking because these files were not used. When linking with --whole-archive, I do run into issues with irq_entry.S.

@jovanbulck
Copy link
Owner

I found this very helpful: https://stackoverflow.com/a/66713969

TL;DR

mov foo@GOTPCREL(%rip), %rax loads &foo into RAX. Often this is followed by mov (%rax), %eax or similar to actually get the value of a global variable like int foo;

so in your patch above, lea should be mov followed by another mov if the value is needed (tested this on another branch)

jovanbulck added a commit that referenced this pull request Sep 24, 2024
Elementary PoC works, needs further testing.

Cf #76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants