From e98016263b1e36d0775a1068649f178db2d3d40c Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Fri, 28 Jan 2022 10:54:49 -0800 Subject: [PATCH] kvm/filters: specify exact set of ioctl-s that are allowed * make filters more precise. * create all vCPU-s before installing filters. PiperOrigin-RevId: 424908294 --- pkg/sentry/platform/kvm/BUILD | 1 + pkg/sentry/platform/kvm/bluepill.go | 6 ++- pkg/sentry/platform/kvm/filters.go | 50 +++++++++++++++++++ pkg/sentry/platform/kvm/filters_amd64.go | 33 ++++++++---- pkg/sentry/platform/kvm/filters_arm64.go | 20 ++++---- pkg/sentry/platform/kvm/machine.go | 21 ++++---- pkg/sentry/platform/kvm/machine_amd64.go | 23 +++++---- pkg/sentry/platform/kvm/machine_arm64.go | 17 ------- .../platform/kvm/machine_arm64_unsafe.go | 7 +-- 9 files changed, 111 insertions(+), 67 deletions(-) create mode 100644 pkg/sentry/platform/kvm/filters.go diff --git a/pkg/sentry/platform/kvm/BUILD b/pkg/sentry/platform/kvm/BUILD index b5eccb920..7e769354a 100644 --- a/pkg/sentry/platform/kvm/BUILD +++ b/pkg/sentry/platform/kvm/BUILD @@ -62,6 +62,7 @@ go_library( "bluepill_fault.go", "bluepill_unsafe.go", "context.go", + "filters.go", "filters_amd64.go", "filters_arm64.go", "kvm.go", diff --git a/pkg/sentry/platform/kvm/bluepill.go b/pkg/sentry/platform/kvm/bluepill.go index 5be2215ed..dd2d737ac 100644 --- a/pkg/sentry/platform/kvm/bluepill.go +++ b/pkg/sentry/platform/kvm/bluepill.go @@ -68,11 +68,15 @@ var ( dieTrampolineAddr uintptr ) +// _SYS_KVM_RETURN_TO_HOST is the system call that is used to transition +// to host. +const _SYS_KVM_RETURN_TO_HOST = ^uintptr(0) + // redpill invokes a syscall with -1. // //go:nosplit func redpill() { - unix.RawSyscall(^uintptr(0), 0, 0, 0) + unix.RawSyscall(_SYS_KVM_RETURN_TO_HOST, 0, 0, 0) } // dieHandler is called by dieTrampoline. diff --git a/pkg/sentry/platform/kvm/filters.go b/pkg/sentry/platform/kvm/filters.go new file mode 100644 index 000000000..6c5cba895 --- /dev/null +++ b/pkg/sentry/platform/kvm/filters.go @@ -0,0 +1,50 @@ +// Copyright 2022 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package kvm + +import ( + "golang.org/x/sys/unix" + + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/seccomp" +) + +// SyscallFilters returns syscalls made exclusively by the KVM platform. +func (k *KVM) SyscallFilters() seccomp.SyscallRules { + r := k.archSyscallFilters() + r.Merge(seccomp.SyscallRules{ + unix.SYS_IOCTL: []seccomp.Rule{ + { + seccomp.MatchAny{}, + seccomp.EqualTo(_KVM_RUN), + }, + { + seccomp.MatchAny{}, + seccomp.EqualTo(_KVM_SET_USER_MEMORY_REGION), + }, + }, + unix.SYS_MEMBARRIER: []seccomp.Rule{ + { + seccomp.EqualTo(linux.MEMBARRIER_CMD_PRIVATE_EXPEDITED), + seccomp.EqualTo(0), + }, + }, + unix.SYS_MMAP: {}, + unix.SYS_RT_SIGSUSPEND: {}, + unix.SYS_RT_SIGTIMEDWAIT: {}, + _SYS_KVM_RETURN_TO_HOST: {}, + }) + return r +} diff --git a/pkg/sentry/platform/kvm/filters_amd64.go b/pkg/sentry/platform/kvm/filters_amd64.go index a78be3403..ef348856d 100644 --- a/pkg/sentry/platform/kvm/filters_amd64.go +++ b/pkg/sentry/platform/kvm/filters_amd64.go @@ -21,20 +21,31 @@ import ( "gvisor.dev/gvisor/pkg/seccomp" ) -// SyscallFilters returns syscalls made exclusively by the KVM platform. -func (*KVM) SyscallFilters() seccomp.SyscallRules { +// archSyscallFilters returns arch-specific syscalls made exclusively by the +// KVM platform. +func (k *KVM) archSyscallFilters() seccomp.SyscallRules { return seccomp.SyscallRules{ - unix.SYS_ARCH_PRCTL: {}, - unix.SYS_IOCTL: {}, - unix.SYS_MEMBARRIER: []seccomp.Rule{ + unix.SYS_ARCH_PRCTL: { { - seccomp.EqualTo(linux.MEMBARRIER_CMD_PRIVATE_EXPEDITED), - seccomp.EqualTo(0), + seccomp.EqualTo(linux.ARCH_GET_FS), + }, + { + seccomp.EqualTo(linux.ARCH_GET_GS), + }, + }, + unix.SYS_IOCTL: []seccomp.Rule{ + { + seccomp.MatchAny{}, + seccomp.EqualTo(_KVM_INTERRUPT), + }, + { + seccomp.MatchAny{}, + seccomp.EqualTo(_KVM_NMI), + }, + { + seccomp.MatchAny{}, + seccomp.EqualTo(_KVM_GET_REGS), }, }, - unix.SYS_MMAP: {}, - unix.SYS_RT_SIGSUSPEND: {}, - unix.SYS_RT_SIGTIMEDWAIT: {}, - 0xffffffffffffffff: {}, // KVM uses syscall -1 to transition to host. } } diff --git a/pkg/sentry/platform/kvm/filters_arm64.go b/pkg/sentry/platform/kvm/filters_arm64.go index 4e5b91048..d47a78d8d 100644 --- a/pkg/sentry/platform/kvm/filters_arm64.go +++ b/pkg/sentry/platform/kvm/filters_arm64.go @@ -12,28 +12,26 @@ // See the License for the specific language governing permissions and // limitations under the License. +//go:build arm64 +// +build arm64 + package kvm import ( "golang.org/x/sys/unix" - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/seccomp" ) -// SyscallFilters returns syscalls made exclusively by the KVM platform. -func (*KVM) SyscallFilters() seccomp.SyscallRules { +// archSyscallFilters returns arch-specific syscalls made exclusively by the +// KVM platform. +func (*KVM) archSyscallFilters() seccomp.SyscallRules { return seccomp.SyscallRules{ - unix.SYS_IOCTL: {}, - unix.SYS_MEMBARRIER: []seccomp.Rule{ + unix.SYS_IOCTL: { { - seccomp.EqualTo(linux.MEMBARRIER_CMD_PRIVATE_EXPEDITED), - seccomp.EqualTo(0), + seccomp.MatchAny{}, + seccomp.EqualTo(_KVM_SET_VCPU_EVENTS), }, }, - unix.SYS_MMAP: {}, - unix.SYS_RT_SIGSUSPEND: {}, - unix.SYS_RT_SIGTIMEDWAIT: {}, - 0xffffffffffffffff: {}, // KVM uses syscall -1 to transition to host. } } diff --git a/pkg/sentry/platform/kvm/machine.go b/pkg/sentry/platform/kvm/machine.go index 14c546081..2b71a6e35 100644 --- a/pkg/sentry/platform/kvm/machine.go +++ b/pkg/sentry/platform/kvm/machine.go @@ -68,6 +68,10 @@ type machine struct { // vCPUsByID are the machine vCPUs, can be indexed by the vCPU's ID. vCPUsByID []*vCPU + // usedVCPUs is the number of vCPUs that have been used from the + // vCPUsByID pool. + usedVCPUs int + // maxVCPUs is the maximum number of vCPUs supported by the machine. maxVCPUs int @@ -79,12 +83,6 @@ type machine struct { // usedSlots is the set of used physical addresses (not sorted). usedSlots []uintptr - - // nextID is the next vCPU ID. - nextID uint32 - - // machineArchState is the architecture-specific state. - machineArchState } const ( @@ -160,12 +158,11 @@ type dieState struct { guestRegs userRegs } -// newVCPU creates a returns a new vCPU. +// createVCPU creates and returns a new vCPU. // // Precondition: mu must be held. -func (m *machine) newVCPU() *vCPU { +func (m *machine) createVCPU(id int) *vCPU { // Create the vCPU. - id := int(atomic.AddUint32(&m.nextID, 1) - 1) fd, _, errno := unix.RawSyscall(unix.SYS_IOCTL, uintptr(m.fd), _KVM_CREATE_VCPU, uintptr(id)) if errno != 0 { panic(fmt.Sprintf("error creating new vCPU: %v", errno)) @@ -452,8 +449,10 @@ func (m *machine) Get() *vCPU { } } - // Get a new vCPU (maybe). - if c := m.getNewVCPU(); c != nil { + // Get vCPU from the m.vCPUsByID pool. + if m.usedVCPUs < m.maxVCPUs { + c := m.vCPUsByID[m.usedVCPUs] + m.usedVCPUs++ c.lock() m.vCPUsByTID[tid] = c m.mu.Unlock() diff --git a/pkg/sentry/platform/kvm/machine_amd64.go b/pkg/sentry/platform/kvm/machine_amd64.go index 1f1e98897..9cb921c24 100644 --- a/pkg/sentry/platform/kvm/machine_amd64.go +++ b/pkg/sentry/platform/kvm/machine_amd64.go @@ -21,6 +21,7 @@ import ( "fmt" "math/big" "reflect" + "runtime" "runtime/debug" "golang.org/x/sys/unix" @@ -55,6 +56,14 @@ func (m *machine) initArchState() error { recover() debug.SetPanicOnFault(old) }() + + // Initialize all vCPUs to minimize kvm ioctl-s allowed by seccomp filters. + m.mu.Lock() + for i := 0; i < m.maxVCPUs; i++ { + m.createVCPU(i) + } + m.mu.Unlock() + c := m.Get() defer m.Put(c) bluepill(c) @@ -63,9 +72,6 @@ func (m *machine) initArchState() error { return nil } -type machineArchState struct { -} - type vCPUArchState struct { // PCIDs is the set of PCIDs for this vCPU. // @@ -483,15 +489,10 @@ func (m *machine) getMaxVCPU() { } else { m.maxVCPUs = int(maxVCPUs) } -} - -// getNewVCPU create a new vCPU (maybe) -func (m *machine) getNewVCPU() *vCPU { - if int(m.nextID) < m.maxVCPUs { - c := m.newVCPU() - return c + rCPUs := runtime.GOMAXPROCS(0) + if rCPUs < m.maxVCPUs { + m.maxVCPUs = rCPUs } - return nil } func archPhysicalRegions(physicalRegions []physicalRegion) []physicalRegion { diff --git a/pkg/sentry/platform/kvm/machine_arm64.go b/pkg/sentry/platform/kvm/machine_arm64.go index e7d27f2bc..a4f3cf224 100644 --- a/pkg/sentry/platform/kvm/machine_arm64.go +++ b/pkg/sentry/platform/kvm/machine_arm64.go @@ -19,7 +19,6 @@ package kvm import ( "runtime" - "sync/atomic" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" @@ -29,11 +28,6 @@ import ( "gvisor.dev/gvisor/pkg/sentry/platform" ) -type machineArchState struct { - //initialvCPUs is the machine vCPUs which has initialized but not used - initialvCPUs map[int]*vCPU -} - type vCPUArchState struct { // PCIDs is the set of PCIDs for this vCPU. // @@ -314,14 +308,3 @@ func (m *machine) getMaxVCPU() { } } } - -// getNewVCPU() scan for an available vCPU from initialvCPUs -func (m *machine) getNewVCPU() *vCPU { - for CID, c := range m.initialvCPUs { - if atomic.CompareAndSwapUint32(&c.state, vCPUReady, vCPUUser) { - delete(m.initialvCPUs, CID) - return c - } - } - return nil -} diff --git a/pkg/sentry/platform/kvm/machine_arm64_unsafe.go b/pkg/sentry/platform/kvm/machine_arm64_unsafe.go index e73d5c544..a06408a19 100644 --- a/pkg/sentry/platform/kvm/machine_arm64_unsafe.go +++ b/pkg/sentry/platform/kvm/machine_arm64_unsafe.go @@ -53,12 +53,9 @@ func (m *machine) initArchState() error { // The reason for the difference is that ARM64 and x86_64 have different KVM timer mechanisms. // If we create vCPU dynamically on ARM64, the timer for vCPU would mess up for a short time. // For more detail, please refer to https://github.com/google/gvisor/issues/5739 - m.initialvCPUs = make(map[int]*vCPU) m.mu.Lock() - for int(m.nextID) < m.maxVCPUs-1 { - c := m.newVCPU() - c.state = 0 - m.initialvCPUs[c.id] = c + for i := 0; i < m.maxVCPUs; i++ { + m.createVCPU(i) } m.mu.Unlock() return nil