Repository: cilium/ebpf
Pull request #1630 (2024-12-17 20:41:51)

prog: add ProgramOptions.LogSizeStart to obtain full log after verifier bug
When the verifier hits a bug, it returns EFAULT while clobbering other retcodes. This defeats the buffer growth algorithm, so the user needs a break-glass procedure for obtaining the full verifier log.

Also, reinstate maxVerifierLogSize since the maxu32 >> 2 limit is still in effect in the Linux kernel. This can be removed if the limit is removed one day and someone has a verifier log that's actually this big.

EFAULT can't be reliably triggered in CI, and I'm currently out of ideas for testing this.

Fixes https://github.com/cilium/ebpf/issues/1554


Patch:
diff --git a/internal/epoll/poller.go b/internal/epoll/poller.go
index 72f0ab5..733e839 100644
--- a/internal/epoll/poller.go
+++ b/internal/epoll/poller.go
@@ -168,13 +168,8 @@ func (p *Poller) Wait(events []unix.EpollEvent, deadline time.Time) (int, error)
 	for {
 		timeout := int(-1)
 		if !deadline.IsZero() {
-			msec := time.Until(deadline).Milliseconds()
-			// Deadline is in the past, don't block.
-			msec = max(msec, 0)
-			// Deadline is too far in the future.
-			msec = min(msec, math.MaxInt)
-
-			timeout = int(msec)
+			// Ensure deadline is not in the past and not too far into the future.
+			timeout = int(internal.Between(time.Until(deadline).Milliseconds(), 0, math.MaxInt))
 		}
 
 		n, err := unix.EpollWait(p.epollFd, events, timeout)
diff --git a/internal/math.go b/internal/math.go
index 93a1670..10cde66 100644
--- a/internal/math.go
+++ b/internal/math.go
@@ -10,6 +10,17 @@ func IsPow[I Integer](n I) bool {
 	return n != 0 && (n&(n-1)) == 0
 }
 
+// Between returns the value clamped between a and b.
+func Between[I Integer](val, a, b I) I {
+	lower, upper := a, b
+	if lower > upper {
+		upper, lower = a, b
+	}
+
+	val = min(val, upper)
+	return max(val, lower)
+}
+
 // Integer represents all possible integer types.
 // Remove when x/exp/constraints is moved to the standard library.
 type Integer interface {
diff --git a/prog.go b/prog.go
index 1c97ae3..3767f28 100644
--- a/prog.go
+++ b/prog.go
@@ -51,6 +51,11 @@ const (
 // verifier log.
 const minVerifierLogSize = 64 * 1024
 
+// maxVerifierLogSize is the maximum size of verifier log buffer the kernel
+// will accept before returning EINVAL. May be increased to MaxUint32 in the
+// future, but avoid the unnecessary EINVAL for now.
+const maxVerifierLogSize = math.MaxUint32 >> 2
+
 // ProgramOptions control loading a program into the kernel.
 type ProgramOptions struct {
 	// Bitmap controlling the detail emitted by the kernel's eBPF verifier log.
@@ -70,6 +75,11 @@ type ProgramOptions struct {
 	// attempt at loading the program.
 	LogLevel LogLevel
 
+	// Starting size of the verifier log buffer. If the verifier log is larger
+	// than this size, the buffer will be grown to fit the entire log. Leave at
+	// its default value unless troubleshooting.
+	LogSizeStart uint32
+
 	// Disables the verifier log completely, regardless of other options.
 	LogDisabled bool
 
@@ -401,9 +411,10 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions) (*Program, er
 
 	// The caller requested a specific verifier log level. Set up the log buffer
 	// so that there is a chance of loading the program in a single shot.
+	logSize := internal.Between(opts.LogSizeStart, minVerifierLogSize, maxVerifierLogSize)
 	var logBuf []byte
 	if !opts.LogDisabled && opts.LogLevel != 0 {
-		logBuf = make([]byte, minVerifierLogSize)
+		logBuf = make([]byte, logSize)
 		attr.LogLevel = opts.LogLevel
 		attr.LogSize = uint32(len(logBuf))
 		attr.LogBuf = sys.NewSlicePointer(logBuf)
@@ -437,12 +448,11 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions) (*Program, er
 			attr.LogLevel = LogLevelBranch
 		}
 
-		// Make an educated guess how large the buffer should be. Start
-		// at minVerifierLogSize and then double the size.
-		logSize := uint32(max(len(logBuf)*2, minVerifierLogSize))
-		if int(logSize) < len(logBuf) {
-			return nil, errors.New("overflow while probing log buffer size")
-		}
+		// Make an educated guess how large the buffer should be by multiplying.
+		// Ensure the size doesn't overflow.
+		const factor = 2
+		logSize := internal.Between(logSize, minVerifierLogSize, maxVerifierLogSize/factor)
+		logSize *= factor
 
 		if attr.LogTrueSize != 0 {
 			// The kernel has given us a hint how large the log buffer has to be.
@@ -468,6 +478,12 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions) (*Program, er
 			return nil, fmt.Errorf("load program: %w (MEMLOCK may be too low, consider rlimit.RemoveMemlock)", err)
 		}
 
+	case errors.Is(err, unix.EFAULT):
+		// EFAULT is returned when the kernel hits a verifier bug, and always
+		// overrides ENOSPC, defeating the buffer growth strategy. Warn the user
+		// that they may need to increase the buffer size manually.
+		return nil, fmt.Errorf("load program: %w (hit verifier bug, increase LogSizeStart to fit the log and check dmesg)", err)
+
 	case errors.Is(err, unix.EINVAL):
 		if bytes.Contains(tail, coreBadCall) {
 			err = errBadRelocation


Test patch:
diff --git a/internal/math_test.go b/internal/math_test.go
index 73d6012..68c4471 100644
--- a/internal/math_test.go
+++ b/internal/math_test.go
@@ -7,6 +7,8 @@ import (
 	"slices"
 	"strings"
 	"testing"
+
+	"github.com/go-quicktest/qt"
 )
 
 func TestPow(t *testing.T) {
@@ -51,3 +53,22 @@ func TestIntegerConstraint(t *testing.T) {
 		}
 	}
 }
+
+func TestBetween(t *testing.T) {
+	tests := []struct {
+		val, a, b, r int
+	}{
+		{0, 1, 2, 1},
+		{1, 1, 2, 1},
+		{2, 1, 2, 2},
+		{3, 1, 2, 2},
+		{4, 10, 5, 5},
+		{11, 10, 5, 10},
+	}
+
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("%d between %d and %d", tt.val, tt.a, tt.b), func(t *testing.T) {
+			qt.Assert(t, qt.Equals(Between(tt.val, tt.a, tt.b), tt.r))
+		})
+	}
+}