Skip to content

Commit

Permalink
platform/kvm: refactor handleBluepillFault to reduce stack usage
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 681217279
  • Loading branch information
avagin authored and gvisor-bot committed Oct 2, 2024
1 parent b99fd87 commit ca8d05a
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 76 deletions.
17 changes: 0 additions & 17 deletions pkg/sentry/platform/kvm/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,6 @@ config_setting(
},
)

# @unused
glaze_ignore = [
"seccomp_mmap_dbg.go",
"seccomp_mmap_real.go",
]

# Use either seccomp_mmap_dbg.go or seccomp_mmap_real.go as seccomp_mmap.go.
genrule(
name = "seccomp_mmap",
srcs = select({
":debug_build": ["seccomp_mmap_dbg.go"],
"//conditions:default": ["seccomp_mmap_real.go"],
}),
outs = ["seccomp_mmap_unsafe.go"],
cmd = "cat < $(SRCS) > $(OUTS)",
)

go_library(
name = "kvm",
srcs = [
Expand Down
2 changes: 1 addition & 1 deletion pkg/sentry/platform/kvm/address_space.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (as *addressSpace) mapLocked(addr hostarch.Addr, m hostMapEntry, at hostarc
// not have physical mappings, the KVM module may inject
// spurious exceptions when emulation fails (i.e. it tries to
// emulate because the RIP is pointed at those pages).
as.machine.mapPhysical(physical, length, physicalRegions)
as.machine.mapPhysical(physical, length)

// Install the page table mappings. Note that the ordering is
// important; if the pagetable mappings were installed before
Expand Down
2 changes: 1 addition & 1 deletion pkg/sentry/platform/kvm/bluepill_allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (a *allocator) PhysicalFor(ptes *pagetables.PTEs) uintptr {
//
//go:nosplit
func (a *allocator) LookupPTEs(physical uintptr) *pagetables.PTEs {
virtualStart, physicalStart, _, pr := calculateBluepillFault(physical, physicalRegions)
virtualStart, physicalStart, _, pr := calculateBluepillFault(physical)
if pr == nil {
panic(fmt.Sprintf("LookupPTEs failed for 0x%x", physical)) // escapes: panic.
}
Expand Down
30 changes: 9 additions & 21 deletions pkg/sentry/platform/kvm/bluepill_fault.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ func yield() {
// calculateBluepillFault calculates the fault address range.
//
//go:nosplit
func calculateBluepillFault(physical uintptr, phyRegions []physicalRegion) (virtualStart, physicalStart, length uintptr, pr *physicalRegion) {
func calculateBluepillFault(physical uintptr) (virtualStart, physicalStart, length uintptr, pr *physicalRegion) {
alignedPhysical := physical &^ uintptr(hostarch.PageSize-1)
for i, pr := range phyRegions {
for i, pr := range physicalRegions {
end := pr.physical + pr.length
if physical < pr.physical || physical >= end {
continue
Expand All @@ -63,27 +63,14 @@ func calculateBluepillFault(physical uintptr, phyRegions []physicalRegion) (virt
physicalEnd = end
}
length = physicalEnd - physicalStart
return virtualStart, physicalStart, length, &phyRegions[i]
return virtualStart, physicalStart, length, &physicalRegions[i]
}

return 0, 0, 0, nil
}

// handleBluepillFault handles a physical fault.
//
// The corresponding virtual address is returned. This may throw on error.
//
//go:nosplit
func handleBluepillFault(m *machine, physical uintptr, phyRegions []physicalRegion) (uintptr, bool) {
// Paging fault: we need to map the underlying physical pages for this
// fault. This all has to be done in this function because we're in a
// signal handler context. (We can't call any functions that might
// split the stack.)
virtualStart, physicalStart, length, pr := calculateBluepillFault(physical, phyRegions)
if pr == nil {
return 0, false
}

func (m *machine) mapMemorySlot(virtualStart, physicalStart, length uintptr, readOnly bool) {
// Set the KVM slot.
//
// First, we need to acquire the exclusive right to set a slot. See
Expand All @@ -94,7 +81,7 @@ func handleBluepillFault(m *machine, physical uintptr, phyRegions []physicalRegi
slot = m.nextSlot.Swap(^uint32(0))
}
flags := _KVM_MEM_FLAGS_NONE
if pr.readOnly {
if readOnly {
flags |= _KVM_MEM_READONLY
}
errno := m.setMemoryRegion(int(slot), physicalStart, length, virtualStart, flags)
Expand All @@ -106,7 +93,7 @@ func handleBluepillFault(m *machine, physical uintptr, phyRegions []physicalRegi
// Successfully added region; we can increment nextSlot and
// allow another set to proceed here.
m.nextSlot.Store(slot + 1)
return virtualStart + (physical - physicalStart), true
return
}

// Release our slot (still available).
Expand All @@ -117,16 +104,17 @@ func handleBluepillFault(m *machine, physical uintptr, phyRegions []physicalRegi
// The region already exists. It's possible that we raced with
// another vCPU here. We just revert nextSlot and return true,
// because this must have been satisfied by some other vCPU.
return virtualStart + (physical - physicalStart), true
return
case unix.EINVAL:
throw("set memory region failed; out of slots")
case unix.ENOMEM:
throw("set memory region failed: out of memory")
case unix.EFAULT:
throw("set memory region failed: invalid physical range")
default:
printHex([]byte("set memory region failed:"), uint64(errno))
throw("set memory region failed: unknown reason")
}

panic("unreachable")
throw("unreachable")
}
12 changes: 5 additions & 7 deletions pkg/sentry/platform/kvm/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ func newMachine(vm int) (*machine, error) {
}

// Ensure the physical range is mapped.
m.mapPhysical(physical, length, physicalRegions)
m.mapPhysical(physical, length)
virtual += length
}
}
Expand All @@ -396,7 +396,7 @@ func newMachine(vm int) (*machine, error) {
})
if mapEntireAddressSpace {
for _, r := range physicalRegions {
m.mapPhysical(r.physical, r.length, physicalRegions)
m.mapPhysical(r.physical, r.length)
}
}
enableAsyncPreemption()
Expand Down Expand Up @@ -438,19 +438,17 @@ func (m *machine) hasSlot(physical uintptr) bool {
// This throws on error.
//
//go:nosplit
func (m *machine) mapPhysical(physical, length uintptr, phyRegions []physicalRegion) {
func (m *machine) mapPhysical(physical, length uintptr) {
for end := physical + length; physical < end; {
_, physicalStart, length, pr := calculateBluepillFault(physical, phyRegions)
virtualStart, physicalStart, length, pr := calculateBluepillFault(physical)
if pr == nil {
// Should never happen.
throw("mapPhysical on unknown physical address")
}

// Is this already mapped? Check the usedSlots.
if !m.hasSlot(physicalStart) {
if _, ok := handleBluepillFault(m, physical, phyRegions); !ok {
throw("handleBluepillFault failed")
}
m.mapMemorySlot(virtualStart, physicalStart, length, pr.readOnly)
}

// Move to the next chunk.
Expand Down
6 changes: 5 additions & 1 deletion pkg/sentry/platform/kvm/machine_amd64_unsafe.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,11 @@ func seccompMmapSyscall(context unsafe.Pointer) (uintptr, uintptr, unix.Errno) {
// MAP_DENYWRITE is deprecated and ignored by kernel. We use it only for seccomp filters.
addr, e := hostsyscall.RawSyscall6(uintptr(ctx.Rax), uintptr(ctx.Rdi), uintptr(ctx.Rsi),
uintptr(ctx.Rdx), uintptr(ctx.R10)|unix.MAP_DENYWRITE, uintptr(ctx.R8), uintptr(ctx.R9))
ctx.Rax = uint64(addr)
if e != 0 {
ctx.Rax = uint64(-e)
} else {
ctx.Rax = uint64(addr)
}

return addr, uintptr(ctx.Rsi), unix.Errno(e)
}
27 changes: 0 additions & 27 deletions pkg/sentry/platform/kvm/seccomp_mmap_dbg.go

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func seccompMmapHandler(context unsafe.Pointer) {
}

// Ensure the physical range is mapped.
m.mapPhysical(physical, length, physicalRegions)
m.mapPhysical(physical, length)
virtual += length
}
}
Expand Down

0 comments on commit ca8d05a

Please sign in to comment.