Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • D dynamorio
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 1,467
    • Issues 1,467
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 44
    • Merge requests 44
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • DynamoRIO
  • dynamorio
  • Merge requests
  • !4702

i#4136: Suspend on static TLS syscalls; avoid swap if no TLS

  • Review changes

  • Download
  • Email patches
  • Plain diff
Merged Derek Bruening requested to merge i4136-drcov-tls-crash into master Jan 30, 2021
  • Overview 2
  • Commits 2
  • Pipelines 0
  • Changes 14

The system call NtSetInformationProcess.ProcessTlsInformation modifies the static TLS pointers in other threads. If DR is swapping static TLS TEB fields, the kernel could modify a thread while it is in DR state, failing to properly update the app state. To solve this, we suspend all other threads and swap them to app state before executing the system call.

Avoids static TLS field swapping if no private libraries are using any slots. Previously, TEB.ThreadLocalStoragePointer was swapped if any client was present at all, regardless of whether any static TLS slots were in use by private libraries. Given that late-loaded static TLS is not supported, we know at gencode creation time whether we have any static TLS slots to make swapping worthwhile. Adds an assert that this holds to help catch this if we later add late-loaded static TLS support.

Adds static TLS to the client.tls test, which exercises the new code, but does not necessarily reproduce a fatal crash from the bug. The suspend solution (without the swap-avoid solution in place yet) was manually tested with drcov on the minimized code calling GetOpenFileName at https://github.com/DynamoRIO/dynamorio/issues/4136#issuecomment-613135121 where a crash happened on initialization prior to drawing the window on every run before the solution was applied.

Fixes #4136 (closed)

Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: i4136-drcov-tls-crash