Skip to content

Commit

Permalink
link: don't use regexp for ID validity checks
Browse files Browse the repository at this point in the history
Replace regular expressions with custom functions checking input strings
byte by byte.

Regular expressions bring in a big library as a dependency, and they
incur some overhead during startup (to compile regexes).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed Apr 27, 2022
1 parent 6a929a7 commit 7f2a37f
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 20 deletions.
39 changes: 30 additions & 9 deletions link/kprobe.go
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
}

// 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 @@ -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
31 changes: 25 additions & 6 deletions link/perf_event.go
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 @@ -373,3 +367,28 @@ var haveBPFLinkPerfEvent = internal.FeatureTest("bpf_link_perf_event", "5.15", f
}
return err
})

// 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
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
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

0 comments on commit 7f2a37f

Please sign in to comment.