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()`.
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.
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 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.
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.
Do not declare the result of the recursive Make call as "phony" to avoid
unnecessary rebuilds. Due to the missing dependency information we of
course still need to always recurse.
Inside `struct shim_handle` `opened` reference counter is used incorrectly.
Additionally at this moment it guards the same resource (handle) as `ref_counter`,
making it obsolete. This patch removes `opened` counter and moves `close_handle`
logic into `put_handle`.
When signal is unmasked and its handler is SIG_IGN, this signal must not
be queued, otherwise such queued but unused signals result in memory leak.
Additionally, SIGCHLD signal can be discarded even if it is unmasked. For
reference, see Linux code (do_notify_parent() @ linux/kernel/signal.c).
Internal LibOS and PAL interfaces use microseconds (us) for timeout
values. However, Linux epoll_wait/epoll_pwait syscalls use milliseconds
(ms) for timeout. Previously, there was a bug in timeout resolution
because epoll_wait() emulation did not convert from ms to us. This
commit fixes this bug and also adds suffixes "_ms" and "_us" to make the
time units used explicit.
Krishnakumar, Sudha