Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

link: do not use regexp pkg #642

Merged
merged 2 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 32 additions & 11 deletions link/kprobe.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"fmt"
"os"
"path/filepath"
"regexp"
"runtime"
"strings"
"sync"
Expand All @@ -27,11 +26,6 @@ var (
value uint64
err error
}{}

// Allow `.` in symbol name. GCC-compiled kernel may change symbol name
// to have a `.isra.$n` suffix, like `udp_send_skb.isra.52`.
// See: https://gcc.gnu.org/gcc-10/changes.html
rgxKprobe = regexp.MustCompile("^[a-zA-Z_][0-9a-zA-Z_.]*$")
)

type probeType uint8
Expand Down Expand Up @@ -144,6 +138,33 @@ func Kretprobe(symbol string, prog *ebpf.Program, opts *KprobeOptions) (Link, er
return lnk, nil
}

kolyshkin marked this conversation as resolved.
Show resolved Hide resolved
// isValidKprobeSymbol implements the equivalent of a regex match
// against "^[a-zA-Z_][0-9a-zA-Z_.]*$".
func isValidKprobeSymbol(s string) bool {
if len(s) < 1 {
return false
}

for i, c := range []byte(s) {
switch {
case c >= 'a' && c <= 'z':
case c >= 'A' && c <= 'Z':
case c == '_':
case i > 0 && c >= '0' && c <= '9':

// Allow `.` in symbol name. GCC-compiled kernel may change symbol name
// to have a `.isra.$n` suffix, like `udp_send_skb.isra.52`.
// See: https://gcc.gnu.org/gcc-10/changes.html
case i > 0 && c == '.':

default:
return false
}
}

return true
}

// kprobe opens a perf event on the given symbol and attaches prog to it.
// If ret is true, create a kretprobe.
func kprobe(symbol string, prog *ebpf.Program, opts *KprobeOptions, ret bool) (*perfEvent, error) {
Expand All @@ -153,7 +174,7 @@ func kprobe(symbol string, prog *ebpf.Program, opts *KprobeOptions, ret bool) (*
if prog == nil {
return nil, fmt.Errorf("prog cannot be nil: %w", errInvalidInput)
}
if !rgxKprobe.MatchString(symbol) {
if !isValidKprobeSymbol(symbol) {
return nil, fmt.Errorf("symbol '%s' must be a valid symbol in /proc/kallsyms: %w", symbol, errInvalidInput)
}
if prog.Type() != ebpf.Kprobe {
Expand Down Expand Up @@ -408,7 +429,7 @@ func createTraceFSProbeEvent(typ probeType, args probeArgs) error {
// the eBPF program itself.
// See Documentation/kprobes.txt for more details.
token = kprobeToken(args)
pe = fmt.Sprintf("%s:%s/%s %s", probePrefix(args.ret), args.group, sanitizedSymbol(args.symbol), token)
pe = fmt.Sprintf("%s:%s/%s %s", probePrefix(args.ret), args.group, sanitizeSymbol(args.symbol), token)
case uprobeType:
// The uprobe_events syntax is as follows:
// p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a probe
Expand Down Expand Up @@ -460,7 +481,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, sanitizedSymbol(symbol))
pe := fmt.Sprintf("-:%s/%s", group, sanitizeSymbol(symbol))
if _, err = f.WriteString(pe); err != nil {
return fmt.Errorf("writing '%s' to '%s': %w", pe, typ.EventsPath(), err)
}
Expand All @@ -471,9 +492,9 @@ func closeTraceFSProbeEvent(typ probeType, group, symbol string) error {
// randomGroup generates a pseudorandom string for use as a tracefs group name.
// Returns an error when the output string would exceed 63 characters (kernel
// limitation), when rand.Read() fails or when prefix contains characters not
// allowed by rgxTraceEvent.
// allowed by isValidTraceID.
func randomGroup(prefix string) (string, error) {
if !rgxTraceEvent.MatchString(prefix) {
if !isValidTraceID(prefix) {
return "", fmt.Errorf("prefix '%s' must be alphanumeric or underscore: %w", prefix, errInvalidInput)
}

Expand Down
33 changes: 26 additions & 7 deletions link/perf_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"os"
"path/filepath"
"regexp"
"runtime"
"strconv"
"strings"
Expand Down Expand Up @@ -45,11 +44,6 @@ import (
var (
tracefsPath = "/sys/kernel/debug/tracing"

// Trace event groups, names and kernel symbols must adhere to this set
// of characters. Non-empty, first character must not be a number, all
// characters must be alphanumeric or underscore.
rgxTraceEvent = regexp.MustCompile("^[a-zA-Z_][0-9a-zA-Z_]*$")

errInvalidInput = errors.New("invalid input")
)

Expand Down Expand Up @@ -275,7 +269,7 @@ func unsafeStringPtr(str string) (unsafe.Pointer, error) {
// 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)
name = sanitizeSymbol(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)
Expand Down Expand Up @@ -373,3 +367,28 @@ var haveBPFLinkPerfEvent = internal.FeatureTest("bpf_link_perf_event", "5.15", f
}
return err
})

kolyshkin marked this conversation as resolved.
Show resolved Hide resolved
// isValidTraceID implements the equivalent of a regex match
// against "^[a-zA-Z_][0-9a-zA-Z_]*$".
//
// Trace event groups, names and kernel symbols must adhere to this set
// of characters. Non-empty, first character must not be a number, all
// characters must be alphanumeric or underscore.
func isValidTraceID(s string) bool {
if len(s) < 1 {
return false
}
for i, c := range []byte(s) {
switch {
case c >= 'a' && c <= 'z':
case c >= 'A' && c <= 'Z':
case c == '_':
case i > 0 && c >= '0' && c <= '9':

default:
return false
}
}

return true
}
8 changes: 4 additions & 4 deletions link/perf_event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ func TestTraceReadID(t *testing.T) {
}
}

func TestTraceEventRegex(t *testing.T) {
var tests = []struct {
func TestTraceValidID(t *testing.T) {
tests := []struct {
name string
in string
fail bool
Expand All @@ -67,8 +67,8 @@ func TestTraceEventRegex(t *testing.T) {
exp = "fail"
}

if rgxTraceEvent.MatchString(tt.in) == tt.fail {
t.Errorf("expected string '%s' to %s regex match", tt.in, exp)
if isValidTraceID(tt.in) == tt.fail {
t.Errorf("expected string '%s' to %s valid ID check", tt.in, exp)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion link/tracepoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func Tracepoint(group, name string, prog *ebpf.Program, opts *TracepointOptions)
if prog == nil {
return nil, fmt.Errorf("prog cannot be nil: %w", errInvalidInput)
}
if !rgxTraceEvent.MatchString(group) || !rgxTraceEvent.MatchString(name) {
if !isValidTraceID(group) || !isValidTraceID(name) {
return nil, fmt.Errorf("group and name '%s/%s' must be alphanumeric or underscore: %w", group, name, errInvalidInput)
}
if prog.Type() != ebpf.TracePoint {
Expand Down
34 changes: 25 additions & 9 deletions link/uprobe.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"fmt"
"os"
"path/filepath"
"regexp"
"strings"
"sync"

"github.com/cilium/ebpf"
Expand All @@ -16,10 +16,6 @@ import (
var (
uprobeEventsPath = filepath.Join(tracefsPath, "uprobe_events")

// 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.
rgxEventSymbol = regexp.MustCompile("[^a-zA-Z0-9]+")

uprobeRetprobeBit = struct {
once sync.Once
value uint64
Expand Down Expand Up @@ -296,7 +292,7 @@ func (ex *Executable) uprobe(symbol string, prog *ebpf.Program, opts *UprobeOpti
}

// Use tracefs if uprobe PMU is missing.
args.symbol = sanitizedSymbol(symbol)
args.symbol = sanitizeSymbol(symbol)
tp, err = tracefsUprobe(args)
if err != nil {
return nil, fmt.Errorf("creating trace event '%s:%s' in tracefs: %w", ex.path, symbol, err)
Expand All @@ -315,9 +311,29 @@ func tracefsUprobe(args probeArgs) (*perfEvent, error) {
return tracefsProbe(uprobeType, args)
}

// sanitizedSymbol replaces every invalid character for the tracefs api with an underscore.
func sanitizedSymbol(symbol string) string {
return rgxEventSymbol.ReplaceAllString(symbol, "_")
// sanitizeSymbol replaces every invalid character for the tracefs api with an underscore.
// It is equivalent to calling regexp.MustCompile("[^a-zA-Z0-9]+").ReplaceAllString("_").
func sanitizeSymbol(s string) string {
var b strings.Builder
ti-mo marked this conversation as resolved.
Show resolved Hide resolved
b.Grow(len(s))
var skip bool
for _, c := range []byte(s) {
switch {
case c >= 'a' && c <= 'z',
c >= 'A' && c <= 'Z',
c >= '0' && c <= '9':
skip = false
b.WriteByte(c)

default:
if !skip {
b.WriteByte('_')
skip = true
}
}
}

return b.String()
}

// uprobeToken creates the PATH:OFFSET(REF_CTR_OFFSET) token for the tracefs api.
Expand Down
10 changes: 6 additions & 4 deletions link/uprobe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func TestUprobeTraceFS(t *testing.T) {

// Prepare probe args.
args := probeArgs{
symbol: sanitizedSymbol(bashSym),
symbol: sanitizeSymbol(bashSym),
path: bashEx.path,
offset: off,
pid: perfAllThreads,
Expand Down Expand Up @@ -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 := sanitizedSymbol(bashSym)
ssym := sanitizeSymbol(bashSym)

pg, _ := randomGroup("ebpftest")
rg, _ := randomGroup("ebpftest")
Expand Down Expand Up @@ -280,14 +280,16 @@ func TestUprobeSanitizedSymbol(t *testing.T) {
expected string
}{
{"readline", "readline"},
{"main.Func", "main_Func"},
{"main.Func123", "main_Func123"},
{"a.....a", "a_a"},
{"./;'{}[]a", "_a"},
{"***xx**xx###", "_xx_xx_"},
{`@P#r$i%v^3*+t)i&k++--`, "_P_r_i_v_3_t_i_k_"},
}

for i, tt := range tests {
t.Run(fmt.Sprint(i), func(t *testing.T) {
sanitized := sanitizedSymbol(tt.symbol)
sanitized := sanitizeSymbol(tt.symbol)
if tt.expected != sanitized {
t.Errorf("Expected sanitized symbol to be '%s', got '%s'", tt.expected, sanitized)
}
Expand Down