From e097384f65af986e9765f0256b57028d0f8e94ca Mon Sep 17 00:00:00 2001 From: Hengqi Chen Date: Thu, 24 Mar 2022 23:05:46 +0800 Subject: [PATCH 1/3] link/kprobe: Sanitize kprobe symbol for tracefs Since we allow `.` in symbol names, we need to sanitize those symbols to please tracefs. Signed-off-by: Hengqi Chen --- link/kprobe.go | 4 ++-- link/perf_event.go | 6 +++++- link/uprobe.go | 12 ++++++------ link/uprobe_test.go | 6 +++--- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/link/kprobe.go b/link/kprobe.go index d88fb6f00..6f8e9f0ba 100644 --- a/link/kprobe.go +++ b/link/kprobe.go @@ -385,7 +385,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 +424,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 fcca8b959..df7c6002a 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 065d2138f..896d9e157 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. diff --git a/link/uprobe_test.go b/link/uprobe_test.go index 046c88476..ae37894a6 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) } From 0814bf4baba5ada017f326836f5989ccc0dacaea Mon Sep 17 00:00:00 2001 From: Hengqi Chen Date: Thu, 24 Mar 2022 23:15:56 +0800 Subject: [PATCH 2/3] link/kprobe: Allow fallback to tracefs on old kernels Centos 8 (with kernel 4.18) has kprobe PMU support, but don't allow `.` in symbol name. Detect such cases and fallback to tracefs interface. Signed-off-by: Hengqi Chen --- link/kprobe.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/link/kprobe.go b/link/kprobe.go index 6f8e9f0ba..6a84d965c 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. From 5f13ae34ff30e1b573efc3bbf2dd2f6479181607 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Wed, 30 Mar 2022 18:29:55 +0200 Subject: [PATCH 3/3] link: test - attach k(ret)probe to optimized kernel symbols containing dots These are all local symbols that visible in /proc/kallsyms and traceable by kprobes. Make sure the library supports attaching to these. Signed-off-by: Timo Beckers --- link/kprobe_test.go | 63 +++++++++++++++++++++++++++++++++------------ link/link_test.go | 6 ++--- 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/link/kprobe_test.go b/link/kprobe_test.go index 4b19f7a01..444033cd1 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 ae4880220..f8142fd0c 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 {