Skip to content

Detect and handle death by signals.#1722

Merged
sporksmith merged 3 commits intoshadow:mainfrom
sporksmith:fatal-signals
Oct 15, 2021
Merged

Detect and handle death by signals.#1722
sporksmith merged 3 commits intoshadow:mainfrom
sporksmith:fatal-signals

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

  • Fix preload mode's sigsegv handler to reraise sigsegv for segv's that
    weren't due to rdtsc.
  • Fix preload mode to detect and cleanly handle death as a result of
    asking the managed process to execute a native syscall (such as
    delivering a fatal signal to itself).

Fixes #1715

@sporksmith sporksmith self-assigned this Oct 15, 2021
@github-actions github-actions bot added Component: Build Build/install tools and dependencies Component: Libraries Support functions like LD_PRELOAD and logging Component: Main Composing the core Shadow executable Component: Testing Unit and integration tests and frameworks labels Oct 15, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 15, 2021

Codecov Report

Merging #1722 (2fb0bf2) into main (ff50d35) will increase coverage by 0.00%.
The diff coverage is 15.62%.

❗ Current head 2fb0bf2 differs from pull request most recent head 9f0363b. Consider uploading reports for the commit 9f0363b to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1722   +/-   ##
=======================================
  Coverage   54.46%   54.47%           
=======================================
  Files         146      148    +2     
  Lines       18941    18967   +26     
  Branches     4551     4556    +5     
=======================================
+ Hits        10317    10333   +16     
- Misses       5871     5881   +10     
  Partials     2753     2753           
Flag Coverage Δ
tests 54.47% <15.62%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/lib/shim/shim_rdtsc.c 8.16% <0.00%> (-2.37%) ⬇️
src/test/exit/test_exit_abort.c 0.00% <0.00%> (ø)
src/test/exit/test_exit_sigsegv.c 0.00% <0.00%> (ø)
src/main/host/thread_preload.c 55.33% <38.46%> (-1.37%) ⬇️
src/lib/tsc/tsc.c 32.39% <0.00%> (-1.41%) ⬇️
src/main/utility/utility.c 13.20% <0.00%> (-0.48%) ⬇️
src/lib/libc_preload/syscall_wrappers.c 17.77% <0.00%> (-0.31%) ⬇️
src/main/host/network_interface.c 71.63% <0.00%> (-0.29%) ⬇️
src/main/host/descriptor/file.c 29.71% <0.00%> (-0.19%) ⬇️
src/lib/tsc/tsc_test.c 30.37% <0.00%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff50d35...9f0363b. Read the comment docs.

@sporksmith
Copy link
Copy Markdown
Contributor Author

Oops; investigating the failure. It's dying - there's a brief window on clone where both threads are allowed to run concurrently. My best guess so far is that we end up having 2 parallel rdtsc's, but since the signal handler is per-process, one of them is just dying. Might have to use the thread-local approach after all

@sporksmith
Copy link
Copy Markdown
Contributor Author

Was able to confirm that we were getting a segv due to an rdtsc instruction in a newly cloned thread, and that at least on debian-10 clone is called with CLONE_SIGHAND, so probably is the race condition I was worried about. Went with the thread-local approach after all.

* Fix preload mode's sigsegv handler to reraise sigsegv for segv's that
weren't due to rdtsc.
* Fix preload mode to detect and cleanly handle death as a result of
asking the managed process to execute a native syscall (such as
delivering a fatal signal to itself).

Fixes shadow#1715
@sporksmith sporksmith disabled auto-merge October 15, 2021 18:37
@sporksmith sporksmith enabled auto-merge October 15, 2021 18:37
@sporksmith sporksmith disabled auto-merge October 15, 2021 18:38
@sporksmith sporksmith enabled auto-merge (squash) October 15, 2021 18:38
@sporksmith sporksmith merged commit 17af5db into shadow:main Oct 15, 2021
@sporksmith sporksmith deleted the fatal-signals branch October 15, 2021 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Build Build/install tools and dependencies Component: Libraries Support functions like LD_PRELOAD and logging Component: Main Composing the core Shadow executable Component: Testing Unit and integration tests and frameworks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shadow deadlocks when managed process aborts

3 participants