Repository: cilium/ebpf
Pull request #805 (2022-09-27 13:28:49)

Prevent pprof from causing BPF verifier livelocks
Read https://dxuuu.xyz/bpf-go-pprof.html for more context.

If a thread receives a signal while blocked in BPF_PROG_LOAD, the verifier can cooperatively interrupt itself by checking pending signals for the thread and return -EAGAIN from the syscall to request userspace to retry.

When a Go program is built and run with pprof enabled, threads are routinely sent a SIGPROF to make them dump profiling information, which can lead to a runaway reaction if the program takes longer to verify than the interrupt frequency. To prevent this, mask SIGPROF before calling BPF_PROG_LOAD and remove it when done.

@danobi 


Patch:
diff --git a/features/prog.go b/features/prog.go
index f434ca0..a3a3a83 100644
--- a/features/prog.go
+++ b/features/prog.go
@@ -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
diff --git a/go.mod b/go.mod
index cf7c7cb..5da855f 100644
--- a/go.mod
+++ b/go.mod
@@ -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 (
diff --git a/go.sum b/go.sum
index 087ab91..e2db3e9 100644
--- a/go.sum
+++ b/go.sum
@@ -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=
diff --git a/internal/sys/signals.go b/internal/sys/signals.go
new file mode 100644
index 0000000..84d6331
--- /dev/null
+++ b/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()
+
+	if err := unix.PthreadSigmask(unix.SIG_BLOCK, &profSet, nil); err != nil {
+		runtime.UnlockOSThread()
+		panic(fmt.Errorf("masking profiler signal: %w", err))
+	}
+}
+
+// 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
+}
diff --git a/internal/sys/syscall.go b/internal/sys/syscall.go
index 6dde230..3205ede 100644
--- a/internal/sys/syscall.go
+++ b/internal/sys/syscall.go
@@ -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)
diff --git a/internal/unix/signals_generic.go b/internal/unix/signals_generic.go
new file mode 100644
index 0000000..01b9d75
--- /dev/null
+++ b/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
+)
diff --git a/internal/unix/signals_mips.go b/internal/unix/signals_mips.go
new file mode 100644
index 0000000..9d86290
--- /dev/null
+++ b/internal/unix/signals_mips.go
@@ -0,0 +1,8 @@
+//go:build linux && (mips || mipsle || mips64 || mips64le)
+
+package unix
+
+const (
+	SIG_BLOCK   = 1
+	SIG_UNBLOCK = 2
+)
diff --git a/internal/unix/types_linux.go b/internal/unix/types_linux.go
index 9cb2537..2241634 100644
--- a/internal/unix/types_linux.go
+++ b/internal/unix/types_linux.go
@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package unix
 
@@ -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)
diff --git a/internal/unix/types_other.go b/internal/unix/types_other.go
index be3fdeb..7ee0a74 100644
--- a/internal/unix/types_other.go
+++ b/internal/unix/types_other.go
@@ -1,5 +1,4 @@
 //go:build !linux
-// +build !linux
 
 package unix
 
@@ -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
@@ -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


Test patch:
diff --git a/internal/sys/signals_test.go b/internal/sys/signals_test.go
new file mode 100644
index 0000000..ea59306
--- /dev/null
+++ b/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 {
+		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")
+	}
+}