Repository: cilium/ebpf
Pull request #274 (2021-04-07 18:25:47)

Detect kernel version to attach kprobes to pre-5.0 kernels
Before kernel 5.0, kprobes can only attached by providing the correct 'version' argument to BPF_PROG_LOAD. This argument must contain the value calculated during kernel compile time using the `KERNEL_VERSION` macro.

This implementation takes a slightly different approach to detecting the kernel version than other libraries, but I think it's more concise and readable, while also being less fragile. Most importantly, there's nothing explicitly distro-specific about it.

- check `/proc/version_signature` (only exists on Ubuntu kernels, but is the only source of truth that contains the patch level)
- read `uname -v`
- read `uname -r`

Against each source, run a regex that matches x.y(.z) that appear at the start of the string or right after a space. If a source yields a usable version number, return the version and halt the search. The result is cached in package `internal`, so only one lookup is performed over the lifetime of the program.

Best reviewed per commit, as some code was moved around.

@brycekahle thanks for the reminder that this was still needed and for pointing out that the patch level can simply be missing on some kernels.

cc @jrfastab 


Patch:
diff --git a/internal/feature.go b/internal/feature.go
index ec62ed3..c94a2e1 100644
--- a/internal/feature.go
+++ b/internal/feature.go
@@ -98,41 +98,3 @@ func FeatureTest(name, version string, fn FeatureTestFn) func() error {
 		return ft.result
 	}
 }
-
-// A Version in the form Major.Minor.Patch.
-type Version [3]uint16
-
-// NewVersion creates a version from a string like "Major.Minor.Patch".
-//
-// Patch is optional.
-func NewVersion(ver string) (Version, error) {
-	var major, minor, patch uint16
-	n, _ := fmt.Sscanf(ver, "%d.%d.%d", &major, &minor, &patch)
-	if n < 2 {
-		return Version{}, fmt.Errorf("invalid version: %s", ver)
-	}
-	return Version{major, minor, patch}, nil
-}
-
-func (v Version) String() string {
-	if v[2] == 0 {
-		return fmt.Sprintf("v%d.%d", v[0], v[1])
-	}
-	return fmt.Sprintf("v%d.%d.%d", v[0], v[1], v[2])
-}
-
-// Less returns true if the version is less than another version.
-func (v Version) Less(other Version) bool {
-	for i, a := range v {
-		if a == other[i] {
-			continue
-		}
-		return a < other[i]
-	}
-	return false
-}
-
-// Unspecified returns true if the version is all zero.
-func (v Version) Unspecified() bool {
-	return v[0] == 0 && v[1] == 0 && v[2] == 0
-}
diff --git a/internal/unix/types_linux.go b/internal/unix/types_linux.go
index b101c77..24bc76f 100644
--- a/internal/unix/types_linux.go
+++ b/internal/unix/types_linux.go
@@ -178,6 +178,11 @@ func BytePtrFromString(s string) (*byte, error) {
 	return linux.BytePtrFromString(s)
 }
 
+// ByteSliceToString is a wrapper
+func ByteSliceToString(s []byte) string {
+	return linux.ByteSliceToString(s)
+}
+
 // Renameat2 is a wrapper
 func Renameat2(olddirfd int, oldpath string, newdirfd int, newpath string, flags uint) error {
 	return linux.Renameat2(olddirfd, oldpath, newdirfd, newpath, flags)
diff --git a/internal/unix/types_other.go b/internal/unix/types_other.go
index 08c8f2c..714589b 100644
--- a/internal/unix/types_other.go
+++ b/internal/unix/types_other.go
@@ -217,6 +217,7 @@ func PerfEventOpen(attr *PerfEventAttr, pid int, cpu int, groupFd int, flags int
 // Utsname is a wrapper
 type Utsname struct {
 	Release [65]byte
+	Version [65]byte
 }
 
 // Uname is a wrapper
@@ -244,6 +245,11 @@ func BytePtrFromString(s string) (*byte, error) {
 	return nil, errNonLinux
 }
 
+// ByteSliceToString is a wrapper
+func ByteSliceToString(s []byte) string {
+	return ""
+}
+
 // Renameat2 is a wrapper
 func Renameat2(olddirfd int, oldpath string, newdirfd int, newpath string, flags uint) error {
 	return errNonLinux
diff --git a/internal/version.go b/internal/version.go
new file mode 100644
index 0000000..1a678bf
--- /dev/null
+++ b/internal/version.go
@@ -0,0 +1,163 @@
+package internal
+
+import (
+	"fmt"
+	"io/ioutil"
+	"regexp"
+	"sync"
+
+	"github.com/cilium/ebpf/internal/unix"
+)
+
+const (
+	// Version constant used in ELF binaries indicating that the loader needs to
+	// substitute the eBPF program's version with the value of the kernel's
+	// KERNEL_VERSION compile-time macro. Used for compatibility with BCC, gobpf
+	// and RedSift.
+	MagicKernelVersion = 0xFFFFFFFE
+)
+
+var (
+	// Match between one and three decimals separated by dots, with the last
+	// segment (patch level) being optional on some kernels.
+	// The x.y.z string must appear at the start of a string or right after
+	// whitespace to prevent sequences like 'x.y.z-a.b.c' from matching 'a.b.c'.
+	rgxKernelVersion = regexp.MustCompile(`(?:\A|\s)\d{1,3}\.\d{1,3}(?:\.\d{1,3})?`)
+
+	kernelVersion = struct {
+		once    sync.Once
+		version Version
+		err     error
+	}{}
+)
+
+// A Version in the form Major.Minor.Patch.
+type Version [3]uint16
+
+// NewVersion creates a version from a string like "Major.Minor.Patch".
+//
+// Patch is optional.
+func NewVersion(ver string) (Version, error) {
+	var major, minor, patch uint16
+	n, _ := fmt.Sscanf(ver, "%d.%d.%d", &major, &minor, &patch)
+	if n < 2 {
+		return Version{}, fmt.Errorf("invalid version: %s", ver)
+	}
+	return Version{major, minor, patch}, nil
+}
+
+func (v Version) String() string {
+	if v[2] == 0 {
+		return fmt.Sprintf("v%d.%d", v[0], v[1])
+	}
+	return fmt.Sprintf("v%d.%d.%d", v[0], v[1], v[2])
+}
+
+// Less returns true if the version is less than another version.
+func (v Version) Less(other Version) bool {
+	for i, a := range v {
+		if a == other[i] {
+			continue
+		}
+		return a < other[i]
+	}
+	return false
+}
+
+// Unspecified returns true if the version is all zero.
+func (v Version) Unspecified() bool {
+	return v[0] == 0 && v[1] == 0 && v[2] == 0
+}
+
+// Kernel implements the kernel's KERNEL_VERSION macro from linux/version.h.
+// It represents the kernel version and patch level as a single value.
+func (v Version) Kernel() uint32 {
+
+	// Kernels 4.4 and 4.9 have their SUBLEVEL clamped to 255 to avoid
+	// overflowing into PATCHLEVEL.
+	// See kernel commit 9b82f13e7ef3 ("kbuild: clamp SUBLEVEL to 255").
+	s := v[2]
+	if s > 255 {
+		s = 255
+	}
+
+	// Truncate members to uint8 to prevent them from spilling over into
+	// each other when overflowing 8 bits.
+	return uint32(uint8(v[0]))<<16 | uint32(uint8(v[1]))<<8 | uint32(uint8(s))
+}
+
+// KernelVersion returns the version of the currently running kernel.
+func KernelVersion() (Version, error) {
+	kernelVersion.once.Do(func() {
+		kernelVersion.version, kernelVersion.err = detectKernelVersion()
+	})
+
+	if kernelVersion.err != nil {
+		return Version{}, kernelVersion.err
+	}
+	return kernelVersion.version, nil
+}
+
+// detectKernelVersion returns the version of the running kernel. It scans the
+// following sources in order: /proc/version_signature, uname -v, uname -r.
+// In each of those locations, the last-appearing x.y(.z) value is selected
+// for parsing. The first location that yields a usable version number is
+// returned.
+func detectKernelVersion() (Version, error) {
+
+	// Try reading /proc/version_signature for Ubuntu compatibility.
+	// Example format: Ubuntu 4.15.0-91.92-generic 4.15.18
+	// This method exists in the kernel itself, see d18acd15c
+	// ("perf tools: Fix kernel version error in ubuntu").
+	if pvs, err := ioutil.ReadFile("/proc/version_signature"); err == nil {
+		// If /proc/version_signature exists, failing to parse it is an error.
+		// It only exists on Ubuntu, where the real patch level is not obtainable
+		// through any other method.
+		v, err := findKernelVersion(string(pvs))
+		if err != nil {
+			return Version{}, err
+		}
+		return v, nil
+	}
+
+	var uname unix.Utsname
+	if err := unix.Uname(&uname); err != nil {
+		return Version{}, fmt.Errorf("calling uname: %w", err)
+	}
+
+	// Debian puts the version including the patch level in uname.Version.
+	// It is not an error if there's no version number in uname.Version,
+	// as most distributions don't use it. Parsing can continue on uname.Release.
+	// Example format: #1 SMP Debian 4.19.37-5+deb10u2 (2019-08-08)
+	if v, err := findKernelVersion(unix.ByteSliceToString(uname.Version[:])); err == nil {
+		return v, nil
+	}
+
+	// Most other distributions have the full kernel version including patch
+	// level in uname.Release.
+	// Example format: 4.19.0-5-amd64, 5.5.10-arch1-1
+	v, err := findKernelVersion(unix.ByteSliceToString(uname.Release[:]))
+	if err != nil {
+		return Version{}, err
+	}
+
+	return v, nil
+}
+
+// findKernelVersion matches s against rgxKernelVersion and parses the result
+// into a Version. If s contains multiple matches, the last entry is selected.
+func findKernelVersion(s string) (Version, error) {
+	m := rgxKernelVersion.FindAllString(s, -1)
+	if m == nil {
+		return Version{}, fmt.Errorf("no kernel version in string: %s", s)
+	}
+	// Pick the last match of the string in case there are multiple.
+	s = m[len(m)-1]
+
+	v, err := NewVersion(s)
+	if err != nil {
+		return Version{}, fmt.Errorf("parsing version string %s: %w", s, err)
+	}
+
+	return v, nil
+}
diff --git a/link/kprobe.go b/link/kprobe.go
index 07a0dd0..ad3a594 100644
--- a/link/kprobe.go
+++ b/link/kprobe.go
@@ -234,10 +234,11 @@ func createTraceFSKprobeEvent(group, symbol string, ret bool) error {
 	// the eBPF program itself. See Documentation/kprobes.txt for more details.
 	pe := fmt.Sprintf("%s:%s/%s %s", kprobePrefix(ret), group, symbol, symbol)
 	_, err = f.WriteString(pe)
-	// Writing to <tracefs>/kprobe_events will return ENOENT when the tracee
-	// kernel symbol does not exist (yet) in the kernel.
-	if os.IsNotExist(err) {
-		return fmt.Errorf("kernel symbol %s not found: %w", symbol, ErrNotSupported)
+	// Since commit 97c753e62e6c, ENOENT is correctly returned instead of EINVAL
+	// when trying to create a kretprobe for a missing symbol. Make sure ENOENT
+	// is returned to the caller.
+	if errors.Is(err, os.ErrNotExist) || errors.Is(err, unix.EINVAL) {
+		return fmt.Errorf("kernel symbol %s not found: %w", symbol, os.ErrNotExist)
 	}
 	if err != nil {
 		return fmt.Errorf("writing '%s' to kprobe_events: %w", pe, err)
diff --git a/prog.go b/prog.go
index 36a3ea2..44041f6 100644
--- a/prog.go
+++ b/prog.go
@@ -143,6 +143,19 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions, btfs btfHandl
 		return nil, fmt.Errorf("can't load %s program on %s", spec.ByteOrder, internal.NativeEndian)
 	}
 
+	// Kernels before 5.0 (6c4fc209fcf9 "bpf: remove useless version check for prog load")
+	// require the version field to be set to the value of the KERNEL_VERSION
+	// macro for kprobe-type programs.
+	// Overwrite Kprobe program version if set to zero or the magic version constant.
+	kv := spec.KernelVersion
+	if spec.Type == Kprobe && (kv == 0 || kv == internal.MagicKernelVersion) {
+		v, err := internal.KernelVersion()
+		if err != nil {
+			return nil, fmt.Errorf("detecting kernel version: %w", err)
+		}
+		kv = v.Kernel()
+	}
+
 	insns := make(asm.Instructions, len(spec.Instructions))
 	copy(insns, spec.Instructions)
 
@@ -165,7 +178,7 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions, btfs btfHandl
 		insCount:           insCount,
 		instructions:       internal.NewSlicePointer(bytecode),
 		license:            internal.NewStringPointer(spec.License),
-		kernelVersion:      spec.KernelVersion,
+		kernelVersion:      kv,
 	}
 
 	if haveObjName() == nil {


Test patch:
diff --git a/internal/feature_test.go b/internal/feature_test.go
index f0c76f4..68b7ed0 100644
--- a/internal/feature_test.go
+++ b/internal/feature_test.go
@@ -63,32 +63,3 @@ func TestFeatureTest(t *testing.T) {
 		t.Error("Cached result of unsuccessful execution")
 	}
 }
-
-func TestVersion(t *testing.T) {
-	a, err := NewVersion("1.2")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	b, err := NewVersion("2.2.1")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if !a.Less(b) {
-		t.Error("A should be less than B")
-	}
-
-	if b.Less(a) {
-		t.Error("B shouldn't be less than A")
-	}
-
-	v200 := Version{2, 0, 0}
-	if !a.Less(v200) {
-		t.Error("1.2.1 should not be less than 2.0.0")
-	}
-
-	if v200.Less(a) {
-		t.Error("2.0.0 should not be less than 1.2.1")
-	}
-}
diff --git a/internal/testutils/feature.go b/internal/testutils/feature.go
index e319086..c6d3495 100644
--- a/internal/testutils/feature.go
+++ b/internal/testutils/feature.go
@@ -1,38 +1,18 @@
 package testutils
 
 import (
-	"bytes"
 	"errors"
-	"sync"
 	"testing"
 
 	"github.com/cilium/ebpf/internal"
-	"github.com/cilium/ebpf/internal/unix"
-)
-
-var (
-	kernelVersionOnce sync.Once
-	kernelVersion     internal.Version
 )
 
 func mustKernelVersion() internal.Version {
-	kernelVersionOnce.Do(func() {
-		var uname unix.Utsname
-		err := unix.Uname(&uname)
-		if err != nil {
-			panic(err)
-		}
-
-		end := bytes.IndexByte(uname.Release[:], 0)
-		release := string(uname.Release[:end])
-
-		kernelVersion, err = internal.NewVersion(release)
-		if err != nil {
-			panic(err)
-		}
-	})
-
-	return kernelVersion
+	v, err := internal.KernelVersion()
+	if err != nil {
+		panic(err)
+	}
+	return v
 }
 
 func CheckFeatureTest(t *testing.T, fn func() error) {
diff --git a/internal/version_test.go b/internal/version_test.go
new file mode 100644
index 0000000..95838e0
--- /dev/null
+++ b/internal/version_test.go
@@ -0,0 +1,93 @@
+package internal
+
+import (
+	"testing"
+)
+
+func TestVersion(t *testing.T) {
+	a, err := NewVersion("1.2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	b, err := NewVersion("2.2.1")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !a.Less(b) {
+		t.Error("A should be less than B")
+	}
+
+	if b.Less(a) {
+		t.Error("B shouldn't be less than A")
+	}
+
+	v200 := Version{2, 0, 0}
+	if !a.Less(v200) {
+		t.Error("1.2.1 should not be less than 2.0.0")
+	}
+
+	if v200.Less(a) {
+		t.Error("2.0.0 should not be less than 1.2.1")
+	}
+}
+
+func TestKernelVersion(t *testing.T) {
+	// Kernels 4.4 and 4.9 have a SUBLEVEL of over 255 and clamp it to 255.
+	// In our implementation, the other version segments are truncated.
+	if v, want := (Version{256, 256, 256}), uint32(255); v.Kernel() != want {
+		t.Errorf("256.256.256 should result in a kernel version of %d, got: %d", want, v.Kernel())
+	}
+
+	// Known good version.
+	if v, want := (Version{4, 9, 128}), uint32(264576); v.Kernel() != want {
+		t.Errorf("4.9.1 should result in a kernel version of %d, got: %d", want, v.Kernel())
+	}
+}
+
+func TestVersionDetection(t *testing.T) {
+	var tests = []struct {
+		name string
+		s    string
+		v    Version
+		err  bool
+	}{
+		{"ubuntu version_signature", "Ubuntu 4.15.0-91.92-generic 4.15.18", Version{4, 15, 18}, false},
+		{"debian uname version", "#1 SMP Debian 4.19.37-5+deb10u2 (2019-08-08)", Version{4, 19, 37}, false},
+		{"debian uname release (missing patch)", "4.19.0-5-amd64", Version{4, 19, 0}, false},
+		{"debian uname all", "Linux foo 5.6.0-0.bpo.2-amd64 #1 SMP Debian 5.6.14-2~bpo10+1 (2020-06-09) x86_64 GNU/Linux", Version{5, 6, 14}, false},
+		{"debian custom uname version", "#1577309 SMP Thu Dec 31 08:32:02 UTC 2020", Version{}, true},
+		{"debian custom uname release (missing patch)", "4.19-ovh-xxxx-std-ipv6-64", Version{4, 19, 0}, false},
+		{"arch uname version", "#1 SMP PREEMPT Thu, 11 Mar 2021 21:27:06 +0000", Version{}, true},
+		{"arch uname release", "5.5.10-arch1-1", Version{5, 5, 10}, false},
+		{"alpine uname version", "#1-Alpine SMP Thu Jan 23 10:58:18 UTC 2020", Version{}, true},
+		{"alpine uname release", "4.14.167-0-virt", Version{4, 14, 167}, false},
+		{"fedora uname version", "#1 SMP Tue May 14 18:22:28 UTC 2019", Version{}, true},
+		{"fedora uname release", "5.0.16-100.fc28.x86_64", Version{5, 0, 16}, false},
+		{"centos8 uname version", "#1 SMP Mon Mar 1 17:16:16 UTC 2021", Version{}, true},
+		{"centos8 uname release", "4.18.0-240.15.1.el8_3.x86_64", Version{4, 18, 0}, false},
+		{"devuan uname version", "#1 SMP Debian 4.19.181-1 (2021-03-19)", Version{4, 19, 181}, false},
+		{"devuan uname release", "4.19.0-16-amd64", Version{4, 19, 0}, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			v, err := findKernelVersion(tt.s)
+			if err != nil {
+				if !tt.err {
+					t.Error("unexpected error:", err)
+				}
+				return
+			}
+
+			if tt.err {
+				t.Error("expected error, but got none")
+			}
+
+			if v != tt.v {
+				t.Errorf("unexpected version for string '%s'. got: %v, want: %v", tt.s, v, tt.v)
+			}
+		})
+	}
+}
diff --git a/link/kprobe_test.go b/link/kprobe_test.go
index 0015a5e..472a9f6 100644
--- a/link/kprobe_test.go
+++ b/link/kprobe_test.go
@@ -25,11 +25,6 @@ var (
 )
 
 func TestKprobe(t *testing.T) {
-
-	//TODO: implement kprobe program version rewriting for pre-5.0 kernels.
-	// Requires at least 5.0 (6c4fc209fcf9 "bpf: remove useless version check for prog load")
-	testutils.SkipOnOldKernel(t, "5.0", "lifted version check for kprobes")
-
 	c := qt.New(t)
 
 	prog, err := ebpf.NewProgram(&kprobeSpec)
@@ -47,18 +42,13 @@ func TestKprobe(t *testing.T) {
 	})
 
 	k, err = Kprobe("bogus", prog)
-	c.Assert(errors.Is(err, os.ErrNotExist), qt.IsTrue)
+	c.Assert(errors.Is(err, os.ErrNotExist), qt.IsTrue, qt.Commentf("got error: %s", err))
 	if k != nil {
 		k.Close()
 	}
 }
 
 func TestKretprobe(t *testing.T) {
-
-	//TODO: implement kprobe program version rewriting for pre-5.0 kernels.
-	// Requires at least 5.0 (6c4fc209fcf9 "bpf: remove useless version check for prog load")
-	testutils.SkipOnOldKernel(t, "5.0", "lifted version check for kprobes")
-
 	c := qt.New(t)
 
 	prog, err := ebpf.NewProgram(&kprobeSpec)
@@ -161,35 +151,43 @@ func BenchmarkKprobeCreatePMU(b *testing.B) {
 }
 
 // Test tracefs k(ret)probe creation on all kernel versions.
-// Create two trace events on the same symbol and ensure their IDs differ.
 func TestKprobeTraceFS(t *testing.T) {
 	c := qt.New(t)
 
 	symbol := "printk"
 
-	// Open and close tracefs kprobe, checking all errors.
-	te, err := tracefsKprobe(symbol, false)
+	// Open and close tracefs k(ret)probes, checking all errors.
+	kp, err := tracefsKprobe(symbol, false)
+	c.Assert(err, qt.IsNil)
+	c.Assert(kp.Close(), qt.IsNil)
+	kp, err = tracefsKprobe(symbol, true)
 	c.Assert(err, qt.IsNil)
-	c.Assert(te.Close(), qt.IsNil)
+	c.Assert(kp.Close(), qt.IsNil)
 
-	// Create similar trace events, ensure their IDs differ.
-	te1, err := tracefsKprobe(symbol, false)
+	// Create two identical trace events, ensure their IDs differ.
+	k1, err := tracefsKprobe(symbol, false)
 	c.Assert(err, qt.IsNil)
-	defer te1.Close()
-	c.Assert(te1.progType, qt.Equals, ebpf.Kprobe)
-	c.Assert(te1.tracefsID, qt.Not(qt.Equals), 0)
+	defer k1.Close()
+	c.Assert(k1.progType, qt.Equals, ebpf.Kprobe)
+	c.Assert(k1.tracefsID, qt.Not(qt.Equals), 0)
 
-	te2, err := tracefsKprobe(symbol, false)
+	k2, err := tracefsKprobe(symbol, false)
 	c.Assert(err, qt.IsNil)
-	defer te2.Close()
-	c.Assert(te2.progType, qt.Equals, ebpf.Kprobe)
-	c.Assert(te2.tracefsID, qt.Not(qt.Equals), 0)
+	defer k2.Close()
+	c.Assert(k2.progType, qt.Equals, ebpf.Kprobe)
+	c.Assert(k2.tracefsID, qt.Not(qt.Equals), 0)
 
-	c.Assert(te1.tracefsID, qt.Not(qt.CmpEquals()), te2.tracefsID)
+	// Compare the kprobes' tracefs IDs.
+	c.Assert(k1.tracefsID, qt.Not(qt.CmpEquals()), k2.tracefsID)
 
-	// Write a kprobe event for a non-existing symbol.
-	err = createTraceFSKprobeEvent("syscalls", "bogus", false)
-	c.Assert(errors.Is(err, ErrNotSupported), qt.IsTrue)
+	// Write a k(ret)probe event for a non-existing symbol.
+	err = createTraceFSKprobeEvent("testgroup", "bogus", false)
+	c.Assert(errors.Is(err, os.ErrNotExist), qt.IsTrue, qt.Commentf("got error: %s", err))
+
+	// A kernel bug was fixed in 97c753e62e6c where EINVAL was returned instead
+	// of ENOENT, but only for kretprobes.
+	err = createTraceFSKprobeEvent("testgroup", "bogus", true)
+	c.Assert(errors.Is(err, os.ErrNotExist), qt.IsTrue, qt.Commentf("got error: %s", err))
 }
 
 func BenchmarkKprobeCreateTraceFS(b *testing.B) {