Repository: cilium/ebpf
Pull request #605 (2022-03-24 15:23:46)

kprobe: fall back to tracefs if kernel doesn't support dots in PMU symbol
Centos 8 (kernel 4.18) has kprobe PMU, but don't allow `.` in symbol name,
add a new option to workaround this.

lmb: @chenhengqi how does the centos kernel fail when `.` is in the name? In the best case we could add an `if errors.Is(err, EFOO)` case that enables the already existing tracefs fallback.
chenhengqi: > @chenhengqi how does the centos kernel fail when `.` is in the name? In the best case we could add an `if errors.Is(err, EFOO)` case that enables the already existing tracefs fallback.

The version of kernel shipped in CentOS 8 Linux is 4.18.

In 4.18, the kernel don't allow dot in symbol name in kprobe PMU case:

https://github.com/torvalds/linux/blob/94710cac0ef4ee177a63b5227664b38c95bbf703/kernel/trace/trace_kprobe.c#L340-L343

Can we always fallback to tracefs interface ? Or check the error against EINVAL ?
ti-mo: No need for feature detection IMO, falling back under a specific error like Lorenz suggested sounds like the best option to me. Is this possible?
chenhengqi: OK, just updated the code as you suggest.
ti-mo: I've added a missing call to `sanitizedSymbol()` in `closeTraceFSProbeEvent()`. Closing a tracefs event created with a sanitized name would of course fail otherwise.

I've also converted `TestK(ret)probe` to be table-driven tests and found some kernel symbols with local visibility that were present in kernel versions ranging from 4.4 to 5.15, so this fallback behaviour can actually be exercised.

Patch:
diff --git a/link/kprobe.go b/link/kprobe.go
index d88fb6f..6a84d96 100644
--- a/link/kprobe.go
+++ b/link/kprobe.go
@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"runtime"
+	"strings"
 	"sync"
 	"unsafe"
 
@@ -263,6 +264,12 @@ func pmuProbe(typ probeType, args probeArgs) (*perfEvent, error) {
 
 	rawFd, err := unix.PerfEventOpen(&attr, args.pid, 0, -1, unix.PERF_FLAG_FD_CLOEXEC)
 
+	// On some old kernels, kprobe PMU doesn't allow `.` in symbol names and
+	// return -EINVAL. Return ErrNotSupported to allow falling back to tracefs.
+	// https://github.com/torvalds/linux/blob/94710cac0ef4/kernel/trace/trace_kprobe.c#L340-L343
+	if errors.Is(err, unix.EINVAL) && strings.Contains(args.symbol, ".") {
+		return nil, fmt.Errorf("symbol '%s': older kernels don't accept dots: %w", args.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.
@@ -385,7 +392,7 @@ func createTraceFSProbeEvent(typ probeType, args probeArgs) error {
 		// subsampling or rate limiting logic can be more accurately implemented in
 		// the eBPF program itself.
 		// See Documentation/kprobes.txt for more details.
-		pe = fmt.Sprintf("%s:%s/%s %s", probePrefix(args.ret), args.group, args.symbol, args.symbol)
+		pe = fmt.Sprintf("%s:%s/%s %s", probePrefix(args.ret), args.group, sanitizedSymbol(args.symbol), args.symbol)
 	case uprobeType:
 		// The uprobe_events syntax is as follows:
 		// p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a probe
@@ -424,7 +431,7 @@ func closeTraceFSProbeEvent(typ probeType, group, symbol string) error {
 
 	// See [k,u]probe_events syntax above. The probe type does not need to be specified
 	// for removals.
-	pe := fmt.Sprintf("-:%s/%s", group, symbol)
+	pe := fmt.Sprintf("-:%s/%s", group, sanitizedSymbol(symbol))
 	if _, err = f.WriteString(pe); err != nil {
 		return fmt.Errorf("writing '%s' to '%s': %w", pe, typ.EventsPath(), err)
 	}
diff --git a/link/perf_event.go b/link/perf_event.go
index fcca8b9..df7c600 100644
--- a/link/perf_event.go
+++ b/link/perf_event.go
@@ -280,8 +280,12 @@ func unsafeStringPtr(str string) (unsafe.Pointer, error) {
 }
 
 // getTraceEventID reads a trace event's ID from tracefs given its group and name.
-// group and name must be alphanumeric or underscore, as required by the kernel.
+// The kernel requires group and name to be alphanumeric or underscore.
+//
+// name automatically has its invalid symbols converted to underscores so the caller
+// can pass a raw symbol name, e.g. a kernel symbol containing dots.
 func getTraceEventID(group, name string) (uint64, error) {
+	name = sanitizedSymbol(name)
 	tid, err := uint64FromFile(tracefsPath, "events", group, name, "id")
 	if errors.Is(err, os.ErrNotExist) {
 		return 0, fmt.Errorf("trace event %s/%s: %w", group, name, os.ErrNotExist)
diff --git a/link/uprobe.go b/link/uprobe.go
index 065d213..896d9e1 100644
--- a/link/uprobe.go
+++ b/link/uprobe.go
@@ -16,9 +16,9 @@ import (
 var (
 	uprobeEventsPath = filepath.Join(tracefsPath, "uprobe_events")
 
-	// rgxUprobeSymbol is used to strip invalid characters from the uprobe symbol
+	// rgxEventSymbol is used to strip invalid characters from the [k,u]probe symbol
 	// as they are not allowed to be used as the EVENT token in tracefs.
-	rgxUprobeSymbol = regexp.MustCompile("[^a-zA-Z0-9]+")
+	rgxEventSymbol = regexp.MustCompile("[^a-zA-Z0-9]+")
 
 	uprobeRetprobeBit = struct {
 		once  sync.Once
@@ -296,7 +296,7 @@ func (ex *Executable) uprobe(symbol string, prog *ebpf.Program, opts *UprobeOpti
 	}
 
 	// Use tracefs if uprobe PMU is missing.
-	args.symbol = uprobeSanitizedSymbol(symbol)
+	args.symbol = sanitizedSymbol(symbol)
 	tp, err = tracefsUprobe(args)
 	if err != nil {
 		return nil, fmt.Errorf("creating trace event '%s:%s' in tracefs: %w", ex.path, symbol, err)
@@ -315,9 +315,9 @@ func tracefsUprobe(args probeArgs) (*perfEvent, error) {
 	return tracefsProbe(uprobeType, args)
 }
 
-// uprobeSanitizedSymbol replaces every invalid characted for the tracefs api with an underscore.
-func uprobeSanitizedSymbol(symbol string) string {
-	return rgxUprobeSymbol.ReplaceAllString(symbol, "_")
+// sanitizedSymbol replaces every invalid character for the tracefs api with an underscore.
+func sanitizedSymbol(symbol string) string {
+	return rgxEventSymbol.ReplaceAllString(symbol, "_")
 }
 
 // uprobeToken creates the PATH:OFFSET(REF_CTR_OFFSET) token for the tracefs api.


Test patch:
diff --git a/link/kprobe_test.go b/link/kprobe_test.go
index 4b19f7a..444033c 100644
--- a/link/kprobe_test.go
+++ b/link/kprobe_test.go
@@ -13,43 +13,72 @@ import (
 	"github.com/cilium/ebpf/internal/unix"
 )
 
-// Kernel symbol that should be present on all tested kernels.
+// Global symbol, present on all tested kernels.
 var ksym = "vprintk"
 
-func TestKprobe(t *testing.T) {
-	c := qt.New(t)
+// Collection of various symbols present in all tested kernels.
+// Compiler optimizations result in different names for these symbols.
+var symTests = []string{
+	"async_resume.cold",         // marked with 'cold' gcc attribute, unlikely to be executed
+	"echo_char.isra.0",          // function optimized by -fipa-sra
+	"get_buffer.constprop.0",    // optimized function with constant operands
+	"unregister_kprobes.part.0", // function body that was split and partially inlined
+}
 
+func TestKprobe(t *testing.T) {
 	prog := mustLoadProgram(t, ebpf.Kprobe, 0, "")
 
-	k, err := Kprobe(ksym, prog, nil)
-	c.Assert(err, qt.IsNil)
-	defer k.Close()
+	for _, tt := range symTests {
+		t.Run(tt, func(t *testing.T) {
+			k, err := Kprobe(tt, prog, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer k.Close()
+		})
+	}
 
-	testLink(t, k, prog)
+	c := qt.New(t)
 
-	k, err = Kprobe("bogus", prog, nil)
-	c.Assert(errors.Is(err, os.ErrNotExist), qt.IsTrue, qt.Commentf("got error: %s", err))
+	k, err := Kprobe("bogus", prog, nil)
+	c.Assert(err, qt.ErrorIs, os.ErrNotExist, qt.Commentf("got error: %s", err))
 	if k != nil {
 		k.Close()
 	}
+
+	k, err = Kprobe(ksym, prog, nil)
+	c.Assert(err, qt.IsNil)
+	defer k.Close()
+
+	testLink(t, k, prog)
 }
 
 func TestKretprobe(t *testing.T) {
-	c := qt.New(t)
-
 	prog := mustLoadProgram(t, ebpf.Kprobe, 0, "")
 
-	k, err := Kretprobe(ksym, prog, nil)
-	c.Assert(err, qt.IsNil)
-	defer k.Close()
+	for _, tt := range symTests {
+		t.Run(tt, func(t *testing.T) {
+			k, err := Kretprobe(tt, prog, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer k.Close()
+		})
+	}
 
-	testLink(t, k, prog)
+	c := qt.New(t)
 
-	k, err = Kretprobe("bogus", prog, nil)
-	c.Assert(errors.Is(err, os.ErrNotExist), qt.IsTrue, qt.Commentf("got error: %s", err))
+	k, err := Kretprobe("bogus", prog, nil)
+	c.Assert(err, qt.ErrorIs, os.ErrNotExist, qt.Commentf("got error: %s", err))
 	if k != nil {
 		k.Close()
 	}
+
+	k, err = Kretprobe(ksym, prog, nil)
+	c.Assert(err, qt.IsNil)
+	defer k.Close()
+
+	testLink(t, k, prog)
 }
 
 func TestKprobeErrors(t *testing.T) {
diff --git a/link/link_test.go b/link/link_test.go
index ae48802..f8142fd 100644
--- a/link/link_test.go
+++ b/link/link_test.go
@@ -97,7 +97,7 @@ func testLink(t *testing.T, link Link, prog *ebpf.Program) {
 	}
 	defer os.RemoveAll(tmp)
 
-	t.Run("pinning", func(t *testing.T) {
+	t.Run("link/pinning", func(t *testing.T) {
 		path := filepath.Join(tmp, "link")
 		err = link.Pin(path)
 		testutils.SkipIfNotSupported(t, err)
@@ -123,7 +123,7 @@ func testLink(t *testing.T, link Link, prog *ebpf.Program) {
 		}
 	})
 
-	t.Run("update", func(t *testing.T) {
+	t.Run("link/update", func(t *testing.T) {
 		err := link.Update(prog)
 		testutils.SkipIfNotSupported(t, err)
 		if err != nil {
@@ -142,7 +142,7 @@ func testLink(t *testing.T, link Link, prog *ebpf.Program) {
 		}()
 	})
 
-	t.Run("link_info", func(t *testing.T) {
+	t.Run("link/info", func(t *testing.T) {
 		info, err := link.Info()
 		testutils.SkipIfNotSupported(t, err)
 		if err != nil {
diff --git a/link/uprobe_test.go b/link/uprobe_test.go
index 046c884..ae37894 100644
--- a/link/uprobe_test.go
+++ b/link/uprobe_test.go
@@ -179,7 +179,7 @@ func TestUprobeTraceFS(t *testing.T) {
 
 	// Prepare probe args.
 	args := probeArgs{
-		symbol: uprobeSanitizedSymbol(bashSym),
+		symbol: sanitizedSymbol(bashSym),
 		path:   bashEx.path,
 		offset: off,
 		pid:    perfAllThreads,
@@ -227,7 +227,7 @@ func TestUprobeCreateTraceFS(t *testing.T) {
 	c.Assert(err, qt.IsNil)
 
 	// Sanitize the symbol in order to be used in tracefs API.
-	ssym := uprobeSanitizedSymbol(bashSym)
+	ssym := sanitizedSymbol(bashSym)
 
 	pg, _ := randomGroup("ebpftest")
 	rg, _ := randomGroup("ebpftest")
@@ -287,7 +287,7 @@ func TestUprobeSanitizedSymbol(t *testing.T) {
 
 	for i, tt := range tests {
 		t.Run(fmt.Sprint(i), func(t *testing.T) {
-			sanitized := uprobeSanitizedSymbol(tt.symbol)
+			sanitized := sanitizedSymbol(tt.symbol)
 			if tt.expected != sanitized {
 				t.Errorf("Expected sanitized symbol to be '%s', got '%s'", tt.expected, sanitized)
 			}