Make CloudABI's way of doing TLS more friendly to userspace emulators.
ClosedPublic
Actions

Authored by ed on Apr 5 2016, 10:57 AM.

Details

Reviewers

kib

Commits

rS297613: Make CloudABI's way of doing TLS more friendly to userspace emulators.

Summary

We're currently seeing how hard it would be to run CloudABI binaries on
operating systems cannot be modified easily (Windows, Mac OS X). The
idea is that we want to just run them without any sandboxing. Now
that CloudABI executables are PIE, this is already a bit easier, but TLS
is still problematic:

CloudABI executables want to write to %fs, which typically requires extra system calls by the emulator every time it needs to switch between CloudABI's and its own TLS.

If CloudABI executables overwrite %fs unconditionally, it also becomes harder for the emulator to store a backup of the old value of %fs.

To solve this, let's no longer overwrite %fs, but just %fs:0. As
CloudABI's C library does not use a TCB, this space can now be used by
an emulator to keep track of its internal state. The executable can now
safely overwrite %fs:0, as long as it makes sure that the TCB is copied
over to the new TLS area.

Ensure that there is an initial TLS area set up when the process starts,
only containing a bogus TCB. We don't really care about its contents on
FreeBSD.

Test Plan

Both old and new CloudABI binaries still seem to work properly. In the
long run we could consider deorbiting cloudabi_sys_tcb_set().

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ed updated this revision to Diff 14883.Apr 5 2016, 10:57 AM

ed retitled this revision from to Make CloudABI's way of doing TLS more friendly to userspace emulators..

ed updated this object.

ed edited the test plan for this revision. (Show Details)

ed added a reviewer: kib.

Herald added subscribers: emaste, andrew, imp. · View Herald TranscriptApr 5 2016, 10:57 AM

ed updated this object.Apr 5 2016, 10:58 AM

kib added inline comments.Apr 6 2016, 10:04 AM

sys/amd64/cloudabi64/cloudabi64_sysvec.c
67 ↗	(On Diff #14883)	'security' people would scream that you just leaked 8 bytes of the kernel stack content to usermode. If thread_setregs() overrides the gap, then you do not need tcbptr and the copyout call there, just do --*stack_base, like you did on arm64.
81 ↗	(On Diff #14883)	'set %fs base to the address of TCB'
164 ↗	(On Diff #14883)	Again, %fs base, not %fs itself.

Fix up comments: %fs -> %fs base.

Fix typo introduced in the previous change.

Thanks for the review!

sys/amd64/cloudabi64/cloudabi64_sysvec.c
67 ↗	(On Diff #14883)	Sorry, but I don't understand this entirely. This function is called on process startup to initialize the process stack. `thread_setregs()` is only called when starting new threads. Are you referring to `proc_setregs()`? I don't really see how this code leaks contents from the kernel stack. The value assigned to `tcbptr` corresponds to the user address at which the TCB is stored, right?