From e671a64c47a9d3df1f3e89f7bf9d891923417853 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Tue, 14 Nov 2023 11:28:45 -0800 Subject: [PATCH] `seccomp`: Add basic `PerArg` optimizations. PiperOrigin-RevId: 582387222 --- pkg/seccomp/seccomp_optimizer.go | 100 +++++++++++++++++++++++++++++++ pkg/seccomp/seccomp_rules.go | 14 +++++ pkg/seccomp/seccomp_test.go | 56 +++++++++++------ 3 files changed, 152 insertions(+), 18 deletions(-) diff --git a/pkg/seccomp/seccomp_optimizer.go b/pkg/seccomp/seccomp_optimizer.go index 9f7a7e6fa..f0bd2e591 100644 --- a/pkg/seccomp/seccomp_optimizer.go +++ b/pkg/seccomp/seccomp_optimizer.go @@ -14,6 +14,11 @@ package seccomp +import ( + "fmt" + "strings" +) + // ruleOptimizerFunc is a function type that can optimize a SyscallRule. // It returns the updated SyscallRule, along with whether any modification // was made. @@ -99,6 +104,86 @@ func convertMatchAllAndXToX(rule SyscallRule) (SyscallRule, bool) { return And(newRules), true } +// nilInPerArgToAnyValue replaces `nil` values in `PerArg` rules with +// `AnyValue`. This isn't really an optimization, but it simplifies the +// logic of other `PerArg` optimizers to not have to handle the `nil` case +// separately from the `AnyValue` case. +func nilInPerArgToAnyValue(rule SyscallRule) (SyscallRule, bool) { + perArg, isPerArg := rule.(PerArg) + if !isPerArg { + return rule, false + } + changed := false + for argNum, valueMatcher := range perArg { + if valueMatcher == nil { + perArg[argNum] = AnyValue{} + changed = true + } + } + return perArg, changed +} + +// convertUselessPerArgToMatchAll looks for `PerArg` rules that match +// anything and replaces them with `MatchAll`. +func convertUselessPerArgToMatchAll(rule SyscallRule) (SyscallRule, bool) { + perArg, isPerArg := rule.(PerArg) + if !isPerArg { + return rule, false + } + for _, valueMatcher := range perArg { + if _, isAnyValue := valueMatcher.(AnyValue); !isAnyValue { + return rule, false + } + } + return MatchAll{}, true +} + +// signature returns a string signature of this `PerArg`. +// This string can be used to identify the behavior of this `PerArg` rule. +func (pa PerArg) signature() string { + var sb strings.Builder + for _, valueMatcher := range pa { + repr := valueMatcher.Repr() + if strings.ContainsRune(repr, ';') { + panic(fmt.Sprintf("ValueMatcher %v (type %T) returned representation %q containing illegal character ';'", valueMatcher, valueMatcher, repr)) + } + sb.WriteString(repr) + sb.WriteRune(';') + } + return sb.String() +} + +// deduplicatePerArgs deduplicates PerArg rules with identical matchers. +// This can happen during filter construction, when rules are added across +// multiple files. +func deduplicatePerArgs[T Or | And](rule SyscallRule) (SyscallRule, bool) { + tRule, isT := rule.(T) + if !isT || len(tRule) < 2 { + return rule, false + } + knownPerArgs := make(map[string]struct{}, len(tRule)) + newRules := make([]SyscallRule, 0, len(tRule)) + changed := false + for _, subRule := range tRule { + subPerArg, subIsPerArg := subRule.(PerArg) + if !subIsPerArg { + newRules = append(newRules, subRule) + continue + } + sig := subPerArg.signature() + if _, isDupe := knownPerArgs[sig]; isDupe { + changed = true + continue + } + knownPerArgs[sig] = struct{}{} + newRules = append(newRules, subPerArg) + } + if !changed { + return rule, false + } + return SyscallRule(T(newRules)), true +} + // optimizeSyscallRuleFuncs losslessly optimizes a SyscallRule using the given // optimization functions. // Optimizers should be ranked in order of importance, with the most @@ -135,5 +220,20 @@ func optimizeSyscallRule(rule SyscallRule) SyscallRule { // linearly scanning through the first (and only) level of rules. convertMatchAllOrXToMatchAll, convertMatchAllAndXToX, + + // Replace all `nil` values in `PerArg` to `AnyValue`, to simplify + // the `PerArg` matchers below. + nilInPerArgToAnyValue, + + // Deduplicate redundant `PerArg`s in Or and And. + // This must come after `nilInPerArgToAnyValue` because it does not + // handle the nil case. + deduplicatePerArgs[Or], + deduplicatePerArgs[And], + + // Remove useless `PerArg` matchers. + // This must come after `nilInPerArgToAnyValue` because it does not + // handle the nil case. + convertUselessPerArgToMatchAll, }) } diff --git a/pkg/seccomp/seccomp_rules.go b/pkg/seccomp/seccomp_rules.go index ce6c9f358..3b6c17080 100644 --- a/pkg/seccomp/seccomp_rules.go +++ b/pkg/seccomp/seccomp_rules.go @@ -57,6 +57,7 @@ type ValueMatcher interface { // Repr returns a string that will be used for asserting equality between // two `ValueMatcher` instances. It must therefore be unique to the // `ValueMatcher` implementation and to its parameters. + // It must not contain the character ";". Repr() string // Render should add rules to the given program that verify the value @@ -623,6 +624,19 @@ type PerArg [7]ValueMatcher // 6 arguments + RIP // instruction pointer. const RuleIP = 6 +// clone returns a copy of this `PerArg`. +func (pa PerArg) clone() PerArg { + return PerArg{ + pa[0], + pa[1], + pa[2], + pa[3], + pa[4], + pa[5], + pa[6], + } +} + // Render implements `SyscallRule.Render`. func (pa PerArg) Render(program *syscallProgram, labelSet *labelSet) { for i, arg := range pa { diff --git a/pkg/seccomp/seccomp_test.go b/pkg/seccomp/seccomp_test.go index 4fc21666a..e3bb4e4a1 100644 --- a/pkg/seccomp/seccomp_test.go +++ b/pkg/seccomp/seccomp_test.go @@ -1221,6 +1221,9 @@ func TestMerge(t *testing.T) { // TestOptimizeSyscallRule tests the behavior of syscall rule optimizers. func TestOptimizeSyscallRule(t *testing.T) { + // av is a shorthand for `AnyValue{}`, used below to keep `PerArg` + // structs short enough to comfortably fit on one line. + av := AnyValue{} for _, test := range []struct { name string rule SyscallRule @@ -1229,8 +1232,8 @@ func TestOptimizeSyscallRule(t *testing.T) { }{ { name: "do nothing to a simple rule", - rule: PerArg{NotEqual(0xff)}, - want: PerArg{NotEqual(0xff)}, + rule: PerArg{NotEqual(0xff), av, av, av, av, av, av}, + want: PerArg{NotEqual(0xff), av, av, av, av, av, av}, }, { name: "flatten Or rule", @@ -1249,12 +1252,12 @@ func TestOptimizeSyscallRule(t *testing.T) { }, }, want: Or{ - PerArg{EqualTo(0x11)}, - PerArg{EqualTo(0x22)}, - PerArg{EqualTo(0x33)}, - PerArg{EqualTo(0x44)}, - PerArg{EqualTo(0x55)}, - PerArg{EqualTo(0x66)}, + PerArg{EqualTo(0x11), av, av, av, av, av, av}, + PerArg{EqualTo(0x22), av, av, av, av, av, av}, + PerArg{EqualTo(0x33), av, av, av, av, av, av}, + PerArg{EqualTo(0x44), av, av, av, av, av, av}, + PerArg{EqualTo(0x55), av, av, av, av, av, av}, + PerArg{EqualTo(0x66), av, av, av, av, av, av}, }, }, { @@ -1274,12 +1277,12 @@ func TestOptimizeSyscallRule(t *testing.T) { }, }, want: And{ - PerArg{NotEqual(0x11)}, - PerArg{NotEqual(0x22)}, - PerArg{NotEqual(0x33)}, - PerArg{NotEqual(0x44)}, - PerArg{NotEqual(0x55)}, - PerArg{NotEqual(0x66)}, + PerArg{NotEqual(0x11), av, av, av, av, av, av}, + PerArg{NotEqual(0x22), av, av, av, av, av, av}, + PerArg{NotEqual(0x33), av, av, av, av, av, av}, + PerArg{NotEqual(0x44), av, av, av, av, av, av}, + PerArg{NotEqual(0x55), av, av, av, av, av, av}, + PerArg{NotEqual(0x66), av, av, av, av, av, av}, }, }, { @@ -1287,14 +1290,14 @@ func TestOptimizeSyscallRule(t *testing.T) { rule: Or{ PerArg{EqualTo(0x11)}, }, - want: PerArg{EqualTo(0x11)}, + want: PerArg{EqualTo(0x11), av, av, av, av, av, av}, }, { name: "simplify And with single rule", rule: And{ PerArg{EqualTo(0x11)}, }, - want: PerArg{EqualTo(0x11)}, + want: PerArg{EqualTo(0x11), av, av, av, av, av, av}, }, { name: "simplify Or with MatchAll", @@ -1328,8 +1331,8 @@ func TestOptimizeSyscallRule(t *testing.T) { PerArg{NotEqual(0x22)}, }, want: And{ - PerArg{NotEqual(0x11)}, - PerArg{NotEqual(0x22)}, + PerArg{NotEqual(0x11), av, av, av, av, av, av}, + PerArg{NotEqual(0x22), av, av, av, av, av, av}, }, }, { @@ -1343,6 +1346,23 @@ func TestOptimizeSyscallRule(t *testing.T) { }, want: MatchAll{}, }, + { + name: "PerArg nil to AnyValue", + rule: PerArg{av, EqualTo(0)}, + optimizers: []ruleOptimizerFunc{ + nilInPerArgToAnyValue, + }, + want: PerArg{av, EqualTo(0), av, av, av, av, av}, + }, + { + name: "Useless PerArg is MatchAll", + rule: PerArg{av, av}, + optimizers: []ruleOptimizerFunc{ + nilInPerArgToAnyValue, + convertUselessPerArgToMatchAll, + }, + want: MatchAll{}, + }, } { t.Run(test.name, func(t *testing.T) { var got SyscallRule