Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent pprof from causing BPF verifier livelocks #805

Merged
merged 5 commits into from Sep 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion features/prog.go
Expand Up @@ -60,7 +60,7 @@ func createProgLoadAttr(pt ebpf.ProgramType, helper asm.BuiltinFunc) (*sys.ProgL
instructions := sys.NewSlicePointer(bytecode)

// Some programs have expected attach types which are checked during the
// BPD_PROG_LOAD syscall.
// BPF_PROG_LOAD syscall.
switch pt {
case ebpf.CGroupSockAddr:
expectedAttachType = ebpf.AttachCGroupInet4Connect
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -5,7 +5,7 @@ go 1.18
require (
github.com/frankban/quicktest v1.14.0
github.com/google/go-cmp v0.5.6
golang.org/x/sys v0.0.0-20220829200755-d48e67d00261
golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6
)

require (
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Expand Up @@ -12,8 +12,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k=
github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 h1:v6hYoSR9T5oet+pMXwUWkbiVqx/63mlHjefrHmxwfeY=
golang.org/x/sys v0.0.0-20220829200755-d48e67d00261/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6 h1:cy1ko5847T/lJ45eyg/7uLprIE/amW5IXxGtEnQdYMI=
golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
82 changes: 82 additions & 0 deletions internal/sys/signals.go
@@ -0,0 +1,82 @@
package sys

import (
"fmt"
"runtime"
"unsafe"

"github.com/cilium/ebpf/internal/unix"
)

var profSet unix.Sigset_t

func init() {
if err := sigsetAdd(&profSet, unix.SIGPROF); err != nil {
panic(fmt.Errorf("creating signal set: %w", err))
}
}

// maskProfilerSignal locks the calling goroutine to its underlying OS thread
// and adds SIGPROF to the thread's signal mask. This prevents pprof from
// interrupting expensive syscalls like e.g. BPF_PROG_LOAD.
//
// The caller must defer sys.UnmaskProfilerSignal() to reverse the operation.
func maskProfilerSignal() {
runtime.LockOSThread()
ti-mo marked this conversation as resolved.
Show resolved Hide resolved

if err := unix.PthreadSigmask(unix.SIG_BLOCK, &profSet, nil); err != nil {
runtime.UnlockOSThread()
panic(fmt.Errorf("masking profiler signal: %w", err))
lmb marked this conversation as resolved.
Show resolved Hide resolved
}
}

// unmaskProfilerSignal removes SIGPROF from the underlying thread's signal
// mask, allowing it to be interrupted for profiling once again.
//
// It also unlocks the current goroutine from its underlying OS thread.
func unmaskProfilerSignal() {
defer runtime.UnlockOSThread()

if err := unix.PthreadSigmask(unix.SIG_UNBLOCK, &profSet, nil); err != nil {
panic(fmt.Errorf("unmasking profiler signal: %w", err))
}
}

const (
wordBytes = int(unsafe.Sizeof(unix.Sigset_t{}.Val[0]))
wordBits = wordBytes * 8

setBytes = int(unsafe.Sizeof(unix.Sigset_t{}))
setBits = setBytes * 8
)

// sigsetAdd adds signal to set.
//
// Note: Sigset_t.Val's value type is uint32 or uint64 depending on the arch.
// This function must be able to deal with both and so must avoid any direct
// references to u32 or u64 types.
func sigsetAdd(set *unix.Sigset_t, signal unix.Signal) error {
if signal < 1 {
return fmt.Errorf("signal %d must be larger than 0", signal)
}
if int(signal) > setBits {
return fmt.Errorf("signal %d does not fit within unix.Sigset_t", signal)
}

// For amd64, runtime.sigaddset() performs the following operation:
// set[(signal-1)/32] |= 1 << ((uint32(signal) - 1) & 31)
//
// This trick depends on sigset being two u32's, causing a signal in the the
// bottom 31 bits to be written to the low word if bit 32 is low, or the high
// word if bit 32 is high.

// Signal is the nth bit in the bitfield.
bit := int(signal - 1)
// Word within the sigset the bit needs to be written to.
word := bit / wordBits

// Write the signal bit into its corresponding word at the corrected offset.
set.Val[word] |= 1 << (bit % wordBits)

return nil
}
61 changes: 61 additions & 0 deletions internal/sys/signals_test.go
@@ -0,0 +1,61 @@
package sys

import (
"runtime"
"testing"

"github.com/cilium/ebpf/internal/unix"
)

func TestSigset(t *testing.T) {
// Type-infer a sigset word. This is a typed uint of 32 or 64 bits depending
// on the target architecture, so we can't use an untyped uint.
zero := unix.Sigset_t{}.Val[0]
words := len(unix.Sigset_t{}.Val)

var want, got unix.Sigset_t
// Flip the first bit of the first word.
if err := sigsetAdd(&got, 1); err != nil {
t.Fatal(err)
}
want.Val[0] = 1
if want != got {
t.Fatalf("expected first word to be 0x%x, got: 0x%x", want, got)
}

// And the last bit of the last word.
if err := sigsetAdd(&got, unix.Signal(setBits)); err != nil {
t.Fatal(err)
}
want.Val[words-1] = ^(^zero >> 1)
if want != got {
t.Fatalf("expected last word to be 0x%x, got: 0x%x", want, got)
}

if err := sigsetAdd(&got, unix.Signal(setBits+1)); err == nil {
t.Fatal("expected out-of-bounds add to be rejected")
}
if err := sigsetAdd(&got, -1); err == nil {
t.Fatal("expected negative signal to be rejected")
}
}

func TestProfilerSignal(t *testing.T) {
// Additional goroutine lock to make the PthreadSigmask below execute on the
// same OS thread as the functions under test. UnlockOSThread needs to be
// called as many times as LockOSThread to unlock the goroutine.
runtime.LockOSThread()
defer runtime.UnlockOSThread()

maskProfilerSignal()
unmaskProfilerSignal()

var old unix.Sigset_t
if err := unix.PthreadSigmask(0, nil, &old); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

t.Fatal("getting old sigmask:", err)
}
var want unix.Sigset_t
if old != want {
t.Fatal("unmask operation didn't result in empty signal mask")
}
}
7 changes: 7 additions & 0 deletions internal/sys/syscall.go
Expand Up @@ -17,6 +17,13 @@ var ENOTSUPP = syscall.Errno(524)
//
// Any pointers contained in attr must use the Pointer type from this package.
func BPF(cmd Cmd, attr unsafe.Pointer, size uintptr) (uintptr, error) {
// Prevent the Go profiler from repeatedly interrupting the verifier,
// which could otherwise lead to a livelock due to receiving EAGAIN.
if cmd == BPF_PROG_LOAD {
maskProfilerSignal()
defer unmaskProfilerSignal()
}

for {
r1, _, errNo := unix.Syscall(unix.SYS_BPF, uintptr(cmd), uintptr(attr), size)
runtime.KeepAlive(attr)
Expand Down
8 changes: 8 additions & 0 deletions internal/unix/signals_generic.go
@@ -0,0 +1,8 @@
//go:build linux && !mips && !mipsle && !mips64 && !mips64le

package unix

const (
SIG_BLOCK = 0
SIG_UNBLOCK = 1
)
8 changes: 8 additions & 0 deletions internal/unix/signals_mips.go
@@ -0,0 +1,8 @@
//go:build linux && (mips || mipsle || mips64 || mips64le)

package unix

const (
SIG_BLOCK = 1
ti-mo marked this conversation as resolved.
Show resolved Hide resolved
SIG_UNBLOCK = 2
)
13 changes: 8 additions & 5 deletions internal/unix/types_linux.go
@@ -1,5 +1,4 @@
//go:build linux
// +build linux

package unix

Expand Down Expand Up @@ -70,21 +69,25 @@ const (
SO_ATTACH_BPF = linux.SO_ATTACH_BPF
SO_DETACH_BPF = linux.SO_DETACH_BPF
SOL_SOCKET = linux.SOL_SOCKET
SIGPROF = linux.SIGPROF
)

// Statfs_t is a wrapper
type Statfs_t = linux.Statfs_t

type Stat_t = linux.Stat_t

// Rlimit is a wrapper
type Rlimit = linux.Rlimit
type Signal = linux.Signal
type Sigset_t = linux.Sigset_t

// Syscall is a wrapper
func Syscall(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err syscall.Errno) {
return linux.Syscall(trap, a1, a2, a3)
}

// PthreadSigmask is a wrapper
func PthreadSigmask(how int, set, oldset *Sigset_t) error {
return linux.PthreadSigmask(how, set, oldset)
}

// FcntlInt is a wrapper
func FcntlInt(fd uintptr, cmd, arg int) (int, error) {
return linux.FcntlInt(fd, cmd, arg)
Expand Down
18 changes: 15 additions & 3 deletions internal/unix/types_other.go
@@ -1,5 +1,4 @@
//go:build !linux
// +build !linux

package unix

Expand Down Expand Up @@ -72,9 +71,12 @@ const (
SO_ATTACH_BPF = 0x32
SO_DETACH_BPF = 0x1b
SOL_SOCKET = 0x1
SIGPROF = 0

SIG_BLOCK = 0
SIG_UNBLOCK = 0
)

// Statfs_t is a wrapper
type Statfs_t struct {
Type int64
Bsize int64
Expand All @@ -92,17 +94,27 @@ type Statfs_t struct {

type Stat_t struct{}

// Rlimit is a wrapper
type Rlimit struct {
Cur uint64
Max uint64
}

type Signal int

type Sigset_t struct {
Val [4]uint64
}

// Syscall is a wrapper
func Syscall(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err syscall.Errno) {
return 0, 0, syscall.Errno(1)
}

// PthreadSigmask is a wrapper
func PthreadSigmask(how int, set, oldset *Sigset_t) error {
return errNonLinux
}

// FcntlInt is a wrapper
func FcntlInt(fd uintptr, cmd, arg int) (int, error) {
return -1, errNonLinux
Expand Down