There was a bug in LibOS code which manifested in heisenbugs in our tests.
The bug happened because `shim_clone.c:clone_implementation_wrapper()` called
`object_wait_with_retry()/DkObjectClose()` before actually setting up a proper
in-enclave TCB. The root cause was:
1. The newly created thread has an in-enclave `FS segment register == 0`
because Graphene-SGX doesn't set up SGX's `TCS.OFSBASGX`, so in-enclave FS
register was equal to `%fs == TCS.OFSBASGX == 0`.
2. `object_wait_with_retry()` checks `PAL_NATIVE_ERRNO` which is implemented
via TCB's `mov %fs:<offset>`.
3. However, `%fs == 0` at this point so there was a SIGBUS while executing
`mov %fs:<offset>`.
The fix (in this commit) is to move `object_wait_with_retry()/DkObjectClose()`
after TCB set-up in `allocate_tls()`.
The new option specifies the file check policy, determining how and which files
can be opened. Previously, there were only two strict options: sgx.trusted_files
and sgx.allowed_files, but they are not flexible enough (e.g., unknown files are
never allowed).
This commit introduces two policies:
- allow_all_but_log allows files other than trusted/allowed files to be opened
but outputs a warning message. This is a convenient way to debug applications.
- strict disallows all files other than trusted/allowed files (just like the
previous logic).
Even though the SysV ABI does not specify the order of argv strings,
some applications (notably Node.js's libuv) assume the compact
encoding of argv where (1) all strings are located adjacently and
(2) in increasing order. This commit reorders argv strings in the
initial user stack in this way.
Applications like OpenVINO sometimes do open("") and expect ENOENT.
This corner case is correctly handled by Linux, so Graphene must have
the exact same semantics. Previously, open("") under Graphene resulted
in success, and subsequent read() failed.
There were some places where put_thread() calls were missing. This
caused reference counter to never reach 0, so that the shim_thread
struct was never freed, thus leaking memory.
We don't need this micro-optimization which just obfuscates the sources
for negligible performance gains. (Un)likeliness of branches in almost
all cases should be derived from profiling, not from hand-written hints,
using profile-guided optimization.
Some hosts do not support IPv6 (e.g., Docker can be configured without
it). In this case, socket() host syscall will return EAFNOSUPPORT.
Some applications rely on this error code (e.g., Redis), so Graphene must
propagate this error code all the way to the application. This commit
makes LibOS and Linux/Linux-SGX PALs aware of this error code.
Previously, exit due to signal produced a return code of 0. This commit
correctly propagates the return code on exiting due to a signal. This
commit also adds two tests and updates graphene-tests submodule to
disable clone02 (this fix exposed an exotic unsupported combination of
clone flags).
- Make assert() a no-op in non-debug builds.
- Use static_assert for compile-time asserts.
- Fix assert() implementation (previous version didn't work for
expressions with types larger than long, it also always printed
`(value:0)`).
- Clean up calls to asserts.
This commit concerns web-server examples: Apache, Nginx, and Lighttpd.
Previously, benchmark-http.sh bash script was located under lighttpd
directory, and Apache and Nginx had symbolic links to it. This is
unintuitive. This commit moves benchmark-http.sh to a newly created
common_tools directory, and all web-server examples have symbolic links
to this new dir.
In test_user_memory(), a memory range is tested via probing of each page
in the range. If the memory page was not allocated, it leads to a
segfault which is captured by the LibOS memfault_upcall() and reported
in the variable tcb.test_range.has_fault. However, the compiler may
optimize away accesses to this variable in test_user_memory(), believing
it is never updated anywhere else. This commit introduces a memory
barrier to prevent this compiler optimization (same for test_user_string()).
Previously, Graphene explicitly incremented refcount of a handle added
to/removed from epoll, in epoll_ctl(EPOLL_CTL_ADD/EPOLL_CTL_DEL).
However, according to epoll(7), closing a file descriptor causes the
FD (handle) to be removed from all epoll sets. In other words,
adding/removing a handle to/from epoll must not count towards refcount
of the handle. Otherwise the handle remains dangling in the epoll set
even if it was close()'d (this particular case led to Nginx segfault).
This commit removes get/put of handle during epoll add/remove.
Typically, listen() is called only once by the application, to mark the
socket as passive for listening for client connections. Thus, LibOS had
a state machine that forbade performing repeated listen() syscalls on
the same socket. However, at least Nginx issues repeated listen's to
adjust the backlog parameter. This commit allows such corner cases.
For detection of SGX/non-SGX (for example in regression tests) always
use the SGX environment variable. To generate launch/EINIT tokens use
the new 'sgx-tokens' Make target.
For this also add the user_address_hole memory range to pal_control.
This is needed to communicate (in a not SGX specific way) the reserved
memory range for the memory gaps around the exec area which can be
inside of the user_address range.
Before argv[0] was treated specially and was changed to a value
from manifest file. Now this happens only if binary was run as
a manifest i.e. `./pal_loader path_to_manifest_file`.
This commit rewrites pal-sgx-get-token and pal-sgx-sign in Python3 and
performs a comprehensive cleanup of the code:
- Outputs are sent to separate files (otherwise they were intermixed
during parallel build)
- aesm_pb2.py is re-generated to Python3
- Numeric constants are replaced with symbolic ones
- Using argparse instead of home-grown argument parsing
- Replacing home-grown helper funcs int_to_bytes(), bytes_to_int(), etc.
with standard functions
The parser logic checks the arguments of syscalls and prints strings in
debug output. Previously, our code base only contained "const char *" as
strings. Recently, we started migration to "const char*" (no-space) style.
This broke the parser logic, so this commit adds an additional check on
what it means for a C type to be a string.
Previously, there was a bug in proc_match_name(): iteration over a
proc_dir array relied on proc_dir.name == proc_dir.nm_ops == NULL.
However, there is no sentinel NULL item in actual arrays, which led to
undefined behavior. This commit changes the for-loop condition to
check proc_dir.size and not rely on the NULL item.
Krishnakumar, Sudha