Skip to content

Commit f585db6

Browse files
konstantin-s-bogomgvisor-bot
authored andcommitted
Provide flag to disable syscall patching (aka syscall fastpath) in Systrap.
First and foremost, this flag is meant to be a debugging tool for Systrap-related issues; very often the first step to debugging Systrap errors is to ask users to disable syscall patching, and this makes that easier. Secondly, it is also meant to be a temporary stop-gap to fix workloads that don't work with syscall patching until the universal solution is implemented to roll-back existing patches to their original state at runtime. Updates #11649. PiperOrigin-RevId: 756520044
1 parent 06acafc commit f585db6

File tree

12 files changed

+51
-12
lines changed

12 files changed

+51
-12
lines changed

pkg/sentry/fsimpl/testutil/kernel.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func Boot() (*kernel.Kernel, error) {
6767
if err != nil {
6868
return nil, fmt.Errorf("creating platform: %v", err)
6969
}
70-
plat, err := platformCtr.New(deviceFile)
70+
plat, err := platformCtr.New(platform.Options{DeviceFile: deviceFile})
7171
if err != nil {
7272
return nil, fmt.Errorf("creating platform: %v", err)
7373
}

pkg/sentry/platform/kvm/kvm.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ func (k *KVM) NewContext(pkgcontext.Context) platform.Context {
183183

184184
type constructor struct{}
185185

186-
func (*constructor) New(f *fd.FD) (platform.Platform, error) {
187-
return New(f, Config{})
186+
func (*constructor) New(opts platform.Options) (platform.Platform, error) {
187+
return New(opts.DeviceFile, Config{})
188188
}
189189

190190
func (*constructor) OpenDevice(devicePath string) (*fd.FD, error) {

pkg/sentry/platform/platform.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,14 +513,25 @@ func (s StaticSeccompInfo) HottestSyscalls() []uintptr {
513513
return s.HotSyscalls
514514
}
515515

516+
// Options is a collection variables and flags for platform initialization.
517+
type Options struct {
518+
// DeviceFile is the device file to use (e.g. /dev/kvm for the KVM
519+
// platform).
520+
DeviceFile *fd.FD
521+
522+
// DisableSyscallPatching controls whether Systrap is allowed to patch
523+
// syscalls invocations sites at runtime.
524+
DisableSyscallPatching bool
525+
}
526+
516527
// Constructor represents a platform type.
517528
type Constructor interface {
518529
// New returns a new platform instance.
519530
//
520531
// Arguments:
521532
//
522-
// * deviceFile - the device file (e.g. /dev/kvm for the KVM platform).
523-
New(deviceFile *fd.FD) (Platform, error)
533+
// * opts - Platform customization bits.
534+
New(opts Options) (Platform, error)
524535

525536
// OpenDevice opens the path to the device used by the platform.
526537
// Passing in an empty string will use the default path for the device,

pkg/sentry/platform/ptrace/ptrace.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func (p *PTrace) NewAddressSpace(any) (platform.AddressSpace, <-chan struct{}, e
260260

261261
type constructor struct{}
262262

263-
func (*constructor) New(*fd.FD) (platform.Platform, error) {
263+
func (*constructor) New(platform.Options) (platform.Platform, error) {
264264
return New()
265265
}
266266

pkg/sentry/platform/systrap/stub_unsafe.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,10 @@ func stubInit() {
306306
*p = uint64(stubContextQueueRegion)
307307
p = (*uint64)(unsafe.Pointer(stubSysmsgStart + uintptr(sysmsg.Sighandler_blob_offset____export_spinning_queue_addr)))
308308
*p = uint64(stubSpinningThreadQueueAddr)
309+
if disableSyscallPatching {
310+
p = (*uint64)(unsafe.Pointer(stubSysmsgStart + uintptr(sysmsg.Sighandler_blob_offset____export_disable_syscall_patching)))
311+
*p = 1
312+
}
309313

310314
prepareSeccompRules(stubSysmsgStart,
311315
stubSysmsgRules, stubSysmsgRulesLen,

pkg/sentry/platform/systrap/subprocess_amd64.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,8 @@ func initChildProcessPPID(initregs *arch.Registers, ppid int32) {
149149
// been generated by the kernel.
150150
// Returns true if the signal info was patched, false otherwise.
151151
func maybePatchSignalInfo(regs *arch.Registers, signalInfo *linux.SignalInfo) bool {
152-
if signalInfo.Addr() < linux.VSyscallStartAddr ||
152+
if disableSyscallPatching ||
153+
signalInfo.Addr() < linux.VSyscallStartAddr ||
153154
signalInfo.Addr() >= linux.VSyscallEndAddr {
154155
return false
155156
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
// sysmsg_lib.c.
4141
struct arch_state __export_arch_state;
4242
uint64_t __export_stub_start;
43+
uint64_t __export_disable_syscall_patching;
4344

4445
long __syscall(long n, long a1, long a2, long a3, long a4, long a5, long a6) {
4546
unsigned long ret;
@@ -251,7 +252,8 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) {
251252
// If a syscall instruction set is "mov sysno, %eax, syscall", it can be
252253
// replaced on a function call which works much faster.
253254
// Look at pkg/sentry/usertrap for more details.
254-
if (siginfo->si_arch == AUDIT_ARCH_X86_64) {
255+
if (__export_disable_syscall_patching == 0 &&
256+
siginfo->si_arch == AUDIT_ARCH_X86_64) {
255257
uint8_t *rip = (uint8_t *)ctx->ptregs.rip;
256258
// FIXME(b/144063246): Even if all five bytes before the syscall
257259
// instruction match the "mov sysno, %eax" instruction, they can be a

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
// sysmsg_lib.c.
3535
struct arch_state __export_arch_state;
3636
uint64_t __export_stub_start;
37+
// Note: This flag doesn't do anything for ARM. See AMD64 equivalent.
38+
uint64_t __export_disable_syscall_patching;
3739

3840
long __syscall(long n, long a1, long a2, long a3, long a4, long a5, long a6) {
3941
// ARM64 syscall interface passes the syscall number in x8 and the 6 arguments

pkg/sentry/platform/systrap/systrap.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,14 @@ var (
113113

114114
// archState stores architecture-specific details used in the platform.
115115
archState sysmsg.ArchState
116+
117+
// disableSyscallPatching controls if Systrap is allowed to patch
118+
// syscall invocation sites.
119+
//
120+
// This is a hacky global and is not toggleable at runtime! Once it has
121+
// been flipped for one Systrap instance, it will apply to all previously
122+
// created and future instances too.
123+
disableSyscallPatching bool
116124
)
117125

118126
// platformContext is an implementation of the platform context.
@@ -241,7 +249,11 @@ func (*Systrap) MinUserAddress() hostarch.Addr {
241249
}
242250

243251
// New returns a new seccomp-based implementation of the platform interface.
244-
func New() (*Systrap, error) {
252+
func New(opts platform.Options) (*Systrap, error) {
253+
if !disableSyscallPatching {
254+
disableSyscallPatching = opts.DisableSyscallPatching
255+
}
256+
245257
if maxSysmsgThreads == 0 {
246258
// CPUID information has been initialized at this point.
247259
archState.Init()
@@ -326,8 +338,8 @@ func (*Systrap) NewContext(ctx pkgcontext.Context) platform.Context {
326338

327339
type constructor struct{}
328340

329-
func (*constructor) New(_ *fd.FD) (platform.Platform, error) {
330-
return New()
341+
func (*constructor) New(opts platform.Options) (platform.Platform, error) {
342+
return New(opts)
331343
}
332344

333345
func (*constructor) OpenDevice(_ string) (*fd.FD, error) {

runsc/boot/loader.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -811,7 +811,10 @@ func createPlatform(conf *config.Config, deviceFile *fd.FD) (platform.Platform,
811811
panic(fmt.Sprintf("invalid platform %s: %s", conf.Platform, err))
812812
}
813813
log.Infof("Platform: %s", conf.Platform)
814-
return p.New(deviceFile)
814+
return p.New(platform.Options{
815+
DeviceFile: deviceFile,
816+
DisableSyscallPatching: conf.Platform == "systrap" && conf.SystrapDisableSyscallPatching,
817+
})
815818
}
816819

817820
func createMemoryFile(appHugePages bool, hostTHP HostTHP) (*pgalloc.MemoryFile, error) {

0 commit comments

Comments
 (0)