Skip to content

Commit 53489c2

Browse files
konstantin-s-bogomgvisor-bot
authored andcommitted
systrap: Initialize stub threads in the same way on both x86 and ARM.
Initializing threads via a custom Systrap entrypoint (as was done on x86 prior to this CL) is incompatible with debuggers, because they set the single-step flag. If a brand new stub thread managed to pick up an already existing context with this flag set, it would immediately invoke the sighandler upon restoring RFLAGS, which would corrupt the stack and instruction pointers. PiperOrigin-RevId: 761250233
1 parent 4a1e993 commit 53489c2

File tree

4 files changed

+15
-35
lines changed

4 files changed

+15
-35
lines changed

pkg/sentry/platform/systrap/subprocess.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1157,7 +1157,10 @@ func (s *subprocess) createSysmsgThread() error {
11571157
if err := p.setRegs(tregs); err != nil {
11581158
panic(fmt.Sprintf("ptrace set regs failed: %v", err))
11591159
}
1160-
archSpecificSysmsgThreadInit(sysThread)
1160+
// Send a fake event to stop the BPF process so that it enters the sighandler.
1161+
if e := hostsyscall.RawSyscallErrno(unix.SYS_TGKILL, uintptr(sysThread.thread.tgid), uintptr(sysThread.thread.tid), uintptr(unix.SIGSEGV)); e != 0 {
1162+
panic(fmt.Sprintf("tkill failed: %v", e))
1163+
}
11611164
// Skip SIGSTOP.
11621165
if e := hostsyscall.RawSyscallErrno(unix.SYS_TGKILL, uintptr(p.tgid), uintptr(p.tid), uintptr(unix.SIGCONT)); e != 0 {
11631166
panic(fmt.Sprintf("tkill failed: %v", e))

pkg/sentry/platform/systrap/subprocess_amd64.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,10 +211,6 @@ func restoreArchSpecificState(ctx *sysmsg.ThreadContext, ac *arch.Context64) {
211211
}
212212

213213
func setArchSpecificRegs(sysThread *sysmsgThread, regs *arch.Registers) {
214-
// Set the start function and initial stack.
215-
regs.PtraceRegs.Rip = uint64(stubSysmsgStart + uintptr(sysmsg.Sighandler_blob_offset____export_start))
216-
regs.PtraceRegs.Rsp = uint64(sysmsg.StackAddrToSyshandlerStack(sysThread.sysmsgPerThreadMemAddr()))
217-
218214
// Set gs_base; this is the only time we set it and we don't expect it to ever
219215
// change for any thread.
220216
regs.Gs_base = sysThread.msg.Self
@@ -223,9 +219,6 @@ func setArchSpecificRegs(sysThread *sysmsgThread, regs *arch.Registers) {
223219
func retrieveArchSpecificState(ctx *sysmsg.ThreadContext, ac *arch.Context64) {
224220
}
225221

226-
func archSpecificSysmsgThreadInit(sysThread *sysmsgThread) {
227-
}
228-
229222
func sigErrorToAccessType(sigError uint64) hostarch.AccessType {
230223
switch {
231224
case sigError&linux.X86_PF_WRITE != 0:

pkg/sentry/platform/systrap/subprocess_arm64.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,13 +190,6 @@ func retrieveArchSpecificState(ctx *sysmsg.ThreadContext, ac *arch.Context64) {
190190
}
191191
}
192192

193-
func archSpecificSysmsgThreadInit(sysThread *sysmsgThread) {
194-
// Send a fake event to stop the BPF process so that it enters the sighandler.
195-
if e := hostsyscall.RawSyscallErrno(unix.SYS_TGKILL, uintptr(sysThread.thread.tgid), uintptr(sysThread.thread.tid), uintptr(unix.SIGSEGV)); e != 0 {
196-
panic(fmt.Sprintf("tkill failed: %v", e))
197-
}
198-
}
199-
200193
func sigErrorToAccessType(sigError uint64) hostarch.AccessType {
201194
return hostarch.ESRAccessType(sigError)
202195
}

pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,17 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) {
206206

207207
if (sysmsg != sysmsg->self) panic(STUB_ERROR_BAD_SYSMSG, 0);
208208
int32_t thread_state = atomic_load(&sysmsg->state);
209+
struct thread_context *ctx = NULL;
210+
enum context_state ctx_state = CONTEXT_STATE_INVALID;
211+
long fs_base = 0;
212+
209213
if (thread_state == THREAD_STATE_INITIALIZING) {
210-
// This thread was interrupted before it even had a context.
211-
return;
214+
// Find a new context and exit to restore it.
215+
init_new_thread();
216+
goto init;
212217
}
213218

214-
struct thread_context *ctx = sysmsg->context;
219+
ctx = sysmsg->context;
215220

216221
// If the current thread is in syshandler, an interrupt has to be postponed,
217222
// because sysmsg can't be changed.
@@ -226,7 +231,7 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) {
226231
return;
227232
}
228233

229-
long fs_base = get_fsbase();
234+
fs_base = get_fsbase();
230235

231236
ctx->signo = signo;
232237
ctx->siginfo = *siginfo;
@@ -242,8 +247,6 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) {
242247
__export_arch_state.fp_len);
243248
}
244249

245-
enum context_state ctx_state = CONTEXT_STATE_INVALID;
246-
247250
switch (signo) {
248251
case SIGSYS: {
249252
ctx_state = CONTEXT_STATE_SYSCALL;
@@ -331,6 +334,8 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) {
331334
}
332335

333336
atomic_store(&ctx->fpstate_changed, 0);
337+
338+
init:
334339
ctx = switch_context_amd64(sysmsg, ctx, ctx_state);
335340
if (fs_base != ctx->ptregs.fs_base) {
336341
set_fsbase(ctx->ptregs.fs_base);
@@ -375,20 +380,6 @@ void __syshandler() {
375380
}
376381
}
377382

378-
void __export_start(struct sysmsg *sysmsg, void *_ucontext) {
379-
init_new_thread();
380-
381-
asm volatile("movq %%gs:0, %0\n" : "=r"(sysmsg) : :);
382-
if (sysmsg->self != sysmsg) {
383-
panic(STUB_ERROR_BAD_SYSMSG, 0);
384-
}
385-
386-
struct thread_context *ctx =
387-
switch_context_amd64(sysmsg, NULL, CONTEXT_STATE_INVALID);
388-
389-
restore_state(sysmsg, ctx, _ucontext);
390-
}
391-
392383
// asm_restore_state is implemented in syshandler_amd64.S
393384
void asm_restore_state();
394385

0 commit comments

Comments
 (0)