From be011b9bfeced56ea58d820e0b27a3b7969cda3a Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Wed, 29 Nov 2023 16:24:24 -0800 Subject: [PATCH] `seccomp`: Optimize half value matchers when possible. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For `MaskedEqual`'s matchers, this looks at the mask being matched against, and simplifies the matcher if that mask is either 0 (in which case any value is allowed), or full bits (in which case there is no need to run an `AND` operation on the bits). Similar thing for `halfNotSet`. Benchmarks: ``` │ before │ after │ │ build-sec │ build-sec vs base │ SentrySystrap 34.10m ± 0% 74.15m ± 0% +117.49% (p=0.000 n=144+145) SentryKVM 142.2m ± 0% 419.5m ± 0% +195.08% (p=0.000 n=144+145) NVProxyIoctl 380.0m ± 0% 1139.7m ± 0% +199.96% (p=0.000 n=144+145) │ before │ after │ │ compression-ratio │ compression-ratio vs base │ SentrySystrap 3.252 ± 0% 3.443 ± 0% +5.87% (p=0.000 n=144+145) SentryKVM 3.318 ± 0% 3.501 ± 0% +5.52% (p=0.000 n=144+145) NVProxyIoctl 3.298 ± 0% 3.504 ± 0% +6.25% (p=0.000 n=144+145) │ before │ after │ │ gen-instr │ gen-instr vs base │ SentrySystrap 1.538k ± 0% 1.618k ± 0% +5.20% (p=0.000 n=144+145) SentryKVM 1.649k ± 0% 1.733k ± 0% +5.09% (p=0.000 n=144+145) NVProxyIoctl 2.233k ± 0% 2.355k ± 0% +5.46% (p=0.000 n=144+145) │ before │ after │ │ opt-instr │ opt-instr vs base │ SentrySystrap 473.0 ± 0% 470.0 ± 0% -0.63% (n=144+145) SentryKVM 497.0 ± 0% 495.0 ± 0% -0.40% (n=144+145) NVProxyIoctl 677.0 ± 0% 672.0 ± 0% -0.74% (n=144+145) │ before │ after │ │ opt-sec │ opt-sec vs base │ SentrySystrap 109.8m ± 0% 109.7m ± 1% ~ (p=0.179 n=144+145) SentryKVM 104.8m ± 0% 103.2m ± 1% -1.44% (p=0.000 n=144+145) NVProxyIoctl 265.7m ± 0% 264.9m ± 1% ~ (p=0.115 n=144+145) ``` PiperOrigin-RevId: 586484636 --- pkg/seccomp/seccomp_optimizer.go | 233 ++++++++++++++++++++++++------- pkg/seccomp/seccomp_test.go | 130 ++++++++++++++--- 2 files changed, 291 insertions(+), 72 deletions(-) diff --git a/pkg/seccomp/seccomp_optimizer.go b/pkg/seccomp/seccomp_optimizer.go index 3da7c7144..e7b1535e4 100644 --- a/pkg/seccomp/seccomp_optimizer.go +++ b/pkg/seccomp/seccomp_optimizer.go @@ -184,6 +184,99 @@ func deduplicatePerArgs[T Or | And](rule SyscallRule) (SyscallRule, bool) { return SyscallRule(T(newRules)), true } +// splitMatchers replaces every `splittableValueMatcher` with a +// `splitMatcher` value matcher instead. +// This enables optimizations that are split-aware to run without +// the need to have logic handling this conversion. +func splitMatchers(rule SyscallRule) (SyscallRule, bool) { + perArg, isPerArg := rule.(PerArg) + if !isPerArg { + return rule, false + } + changed := false + for argNum, valueMatcher := range perArg { + if _, isAlreadySplit := valueMatcher.(splitMatcher); isAlreadySplit { + continue + } + splittableMatcher, isSplittableMatcher := valueMatcher.(splittableValueMatcher) + if !isSplittableMatcher { + continue + } + perArg[argNum] = splittableMatcher.split() + changed = true + } + return perArg, changed +} + +// simplifyHalfValueMatcher may convert a `halfValueMatcher` to a simpler +// (and potentially faster) representation. +func simplifyHalfValueMatcher(hvm halfValueMatcher) halfValueMatcher { + switch v := hvm.(type) { + case halfNotSet: + if v == 0 { + return halfAnyValue{} + } + case halfMaskedEqual: + if v.mask == 0 && v.value == 0 { + return halfAnyValue{} + } + if v.mask == 0xffffffff { + return halfEqualTo(v.value) + } + } + return hvm +} + +// simplifyHalfValueMatchers replace `halfValueMatcher`s with their simplified +// version. +func simplifyHalfValueMatchers(rule SyscallRule) (SyscallRule, bool) { + perArg, isPerArg := rule.(PerArg) + if !isPerArg { + return rule, false + } + changed := false + for i, valueMatcher := range perArg { + sm, isSplitMatcher := valueMatcher.(splitMatcher) + if !isSplitMatcher { + continue + } + if newHigh := simplifyHalfValueMatcher(sm.highMatcher); newHigh.Repr() != sm.highMatcher.Repr() { + sm.highMatcher = newHigh + perArg[i] = sm + changed = true + } + if newLow := simplifyHalfValueMatcher(sm.lowMatcher); newLow.Repr() != sm.lowMatcher.Repr() { + sm.lowMatcher = newLow + perArg[i] = sm + changed = true + } + } + return perArg, changed +} + +// anySplitMatchersToAnyValue converts `splitMatcher`s where both halves +// match any value to a single AnyValue{} rule. +func anySplitMatchersToAnyValue(rule SyscallRule) (SyscallRule, bool) { + perArg, isPerArg := rule.(PerArg) + if !isPerArg { + return rule, false + } + changed := false + for argNum, valueMatcher := range perArg { + sm, isSplitMatcher := valueMatcher.(splitMatcher) + if !isSplitMatcher { + continue + } + _, highIsAny := sm.highMatcher.(halfAnyValue) + _, lowIsAny := sm.lowMatcher.(halfAnyValue) + if highIsAny && lowIsAny { + perArg[argNum] = AnyValue{} + changed = true + } + } + return perArg, changed +} + // invalidValueMatcher is a stand-in `ValueMatcher` with a unique // representation that doesn't look like any legitimate `ValueMatcher`. // Calling any method other than `Repr` will fail. @@ -372,27 +465,54 @@ func extractRepeatedMatchers(rule SyscallRule) (SyscallRule, bool) { return rule, false } -// optimizeSyscallRuleFuncs losslessly optimizes a SyscallRule using the given -// optimization functions. -// Optimizers should be ranked in order of importance, with the most -// important first. -// An optimizer will be exhausted before the next one is ever run. -// Earlier optimizers are re-exhausted if later optimizers cause change. -func optimizeSyscallRuleFuncs(rule SyscallRule, funcs []ruleOptimizerFunc) SyscallRule { - // Instantiate this closure only once, since passing it to (interface - // method) rule.Recurse() causes it to escape. - var recurse func(subRule SyscallRule) SyscallRule - recurse = func(subRule SyscallRule) SyscallRule { - return optimizeSyscallRuleFuncsRecursive(subRule, funcs, recurse) - } - return optimizeSyscallRuleFuncsRecursive(rule, funcs, recurse) +// optimizationRun is a stateful struct tracking the state of an optimization +// over a rule. It may not be used concurrently. +type optimizationRun struct { + // funcs is the list of optimizer functions to run on the rules. + // Optimizers should be ranked in order of importance, with the most + // important first. + // An optimizer will be exhausted before the next one is ever run. + // Earlier optimizers are re-exhausted if later optimizers cause change. + funcs []ruleOptimizerFunc + + // recurseFuncs is a list of closures that correspond one-to-one to `funcs` + // and are suitable for passing to `SyscallRule.Recurse`. They are stored + // here in order to be allocated once, as opposed to escaping if they were + // specified directly as argument to `SyscallRule.Recurse`. + recurseFuncs []func(subRule SyscallRule) SyscallRule + + // changed tracks whether any change has been made in the current pass. + // It is updated as the optimizer runs. + changed bool } -func optimizeSyscallRuleFuncsRecursive(rule SyscallRule, funcs []ruleOptimizerFunc, recurse func(subRule SyscallRule) SyscallRule) SyscallRule { - for changed := true; changed; { - for _, fn := range funcs { - rule.Recurse(recurse) - if rule, changed = fn(rule); changed { +// apply recursively applies `opt.funcs[funcIndex]` to the given `rule`. +// It sets `opt.changed` to true if there has been any change. +func (opt *optimizationRun) apply(rule SyscallRule, funcIndex int) SyscallRule { + rule.Recurse(opt.recurseFuncs[funcIndex]) + if opt.changed { + return rule + } + rule, opt.changed = opt.funcs[funcIndex](rule) + return rule +} + +// optimize losslessly optimizes a SyscallRule using the `optimizationRun`'s +// optimizer functions. +// It may not be called concurrently. +func (opt *optimizationRun) optimize(rule SyscallRule) SyscallRule { + opt.recurseFuncs = make([]func(SyscallRule) SyscallRule, len(opt.funcs)) + for i := range opt.funcs { + funcIndex := i + opt.recurseFuncs[funcIndex] = func(subRule SyscallRule) SyscallRule { + return opt.apply(subRule, funcIndex) + } + } + for opt.changed = true; opt.changed; { + for i := range opt.funcs { + opt.changed = false + rule = opt.apply(rule, i) + if opt.changed { break } } @@ -402,42 +522,53 @@ func optimizeSyscallRuleFuncsRecursive(rule SyscallRule, funcs []ruleOptimizerFu // optimizeSyscallRule losslessly optimizes a `SyscallRule`. func optimizeSyscallRule(rule SyscallRule) SyscallRule { - return optimizeSyscallRuleFuncs(rule, []ruleOptimizerFunc{ - // Convert Or / And rules with a single rule into that single rule. - convertSingleCompoundRuleToThatRule[Or], - convertSingleCompoundRuleToThatRule[And], + return (&optimizationRun{ + funcs: []ruleOptimizerFunc{ + // Convert Or / And rules with a single rule into that single rule. + convertSingleCompoundRuleToThatRule[Or], + convertSingleCompoundRuleToThatRule[And], - // Flatten Or/And rules. - flattenCompoundRules[Or], - flattenCompoundRules[And], + // Flatten Or/And rules. + flattenCompoundRules[Or], + flattenCompoundRules[And], - // Handle MatchAll. This is best done after flattening so that we - // effectively traverse the whole tree to find a MatchAll by just - // linearly scanning through the first (and only) level of rules. - convertMatchAllOrXToMatchAll, - convertMatchAllAndXToX, + // Handle MatchAll. This is best done after flattening so that we + // effectively traverse the whole tree to find a MatchAll by just + // 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, + // 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], + // 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, + // Remove useless `PerArg` matchers. + // This must come after `nilInPerArgToAnyValue` because it does not + // handle the nil case. + convertUselessPerArgToMatchAll, - // Extract repeated argument matchers out of `Or` expressions. - // This must come after `nilInPerArgToAnyValue` because it does not - // handle the nil case. - // This should ideally run late in the list because it does a bunch - // of memory allocations (even in the non-optimizable case), which - // should be avoided unless there is nothing else left to optimize. - extractRepeatedMatchers, - }) + // Replace `ValueMatcher`s that are splittable into their split version. + splitMatchers, + + // Replace `halfValueMatcher`s with their simplified version. + simplifyHalfValueMatchers, + + // Replace `splitMatchers` that match any value with `AnyValue`. + anySplitMatchersToAnyValue, + + // Extract repeated argument matchers out of `Or` expressions. + // This must come after `nilInPerArgToAnyValue` because it does not + // handle the nil case. + // This should ideally run late in the list because it does a bunch + // of memory allocations (even in the non-optimizable case), which + // should be avoided unless there is nothing else left to optimize. + extractRepeatedMatchers, + }, + }).optimize(rule) } diff --git a/pkg/seccomp/seccomp_test.go b/pkg/seccomp/seccomp_test.go index dbb862a02..612c2a712 100644 --- a/pkg/seccomp/seccomp_test.go +++ b/pkg/seccomp/seccomp_test.go @@ -1225,6 +1225,26 @@ 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{} + + // Some useful constants that are larger than uint32. + const ( + a1 = 0xA1A1A1A1A1A1A1A1 + a2 = 0xA2A2A2A2A2A2A2A2 + b1 = 0xB1B1B1B1B1B1B1B1 + b2 = 0xB2B2B2B2B2B2B2B2 + b3 = 0xB3B3B3B3B3B3B3B3 + c1 = 0xC1C1C1C1C1C1C1C1 + c2 = 0xC2C2C2C2C2C2C2C2 + c3 = 0xC3C3C3C3C3C3C3C3 + d0 = 0xD0D0D0D0D0D0D0D0 + ) + + // split replaces a `splittableValueMatcher` rule with its `splitMatcher` + // version. + s := func(matcher splittableValueMatcher) ValueMatcher { + return matcher.split() + } + for _, test := range []struct { name string rule SyscallRule @@ -1253,12 +1273,12 @@ func TestOptimizeSyscallRule(t *testing.T) { }, }, want: Or{ - 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}, + PerArg{s(EqualTo(0x11)), av, av, av, av, av, av}, + PerArg{s(EqualTo(0x22)), av, av, av, av, av, av}, + PerArg{s(EqualTo(0x33)), av, av, av, av, av, av}, + PerArg{s(EqualTo(0x44)), av, av, av, av, av, av}, + PerArg{s(EqualTo(0x55)), av, av, av, av, av, av}, + PerArg{s(EqualTo(0x66)), av, av, av, av, av, av}, }, }, { @@ -1291,14 +1311,14 @@ func TestOptimizeSyscallRule(t *testing.T) { rule: Or{ PerArg{EqualTo(0x11)}, }, - want: PerArg{EqualTo(0x11), av, av, av, av, av, av}, + want: PerArg{s(EqualTo(0x11)), av, av, av, av, av, av}, }, { name: "simplify And with single rule", rule: And{ PerArg{EqualTo(0x11)}, }, - want: PerArg{EqualTo(0x11), av, av, av, av, av, av}, + want: PerArg{s(EqualTo(0x11)), av, av, av, av, av, av}, }, { name: "simplify Or with MatchAll", @@ -1364,26 +1384,94 @@ func TestOptimizeSyscallRule(t *testing.T) { }, want: MatchAll{}, }, + { + name: "halfValueMatchers are simplified", + rule: PerArg{ + EqualTo(0), + splitMatcher{ + highMatcher: halfNotSet(0), + lowMatcher: halfMaskedEqual{ + mask: 0, + value: 0x89abcdef, + }, + }, + splitMatcher{ + highMatcher: halfNotSet(1 << 4), + lowMatcher: halfMaskedEqual{ + mask: 0xffffffff, + value: 0x89abcdef, + }, + }, + splitMatcher{ + highMatcher: halfNotSet(0), + lowMatcher: halfMaskedEqual{ + mask: 0xffffffff, + value: 0x89abcdef, + }, + }, + splitMatcher{ + highMatcher: halfNotSet(0), + lowMatcher: halfMaskedEqual{ + mask: 0, + value: 0, + }, + }, + splitMatcher{ + highMatcher: halfAnyValue{}, + lowMatcher: halfMaskedEqual{ + mask: 0x89abcdef, + value: 0x89abcdef, + }, + }, + }, + want: PerArg{ + s(EqualTo(0)), + splitMatcher{ + highMatcher: halfAnyValue{}, + lowMatcher: halfMaskedEqual{ + mask: 0, + value: 0x89abcdef, + }, + }, + splitMatcher{ + highMatcher: halfNotSet(1 << 4), + lowMatcher: halfEqualTo(0x89abcdef), + }, + splitMatcher{ + highMatcher: halfAnyValue{}, + lowMatcher: halfEqualTo(0x89abcdef), + }, + av, // Both halves are simplified to `halfAnyValue`. + splitMatcher{ + highMatcher: halfAnyValue{}, + lowMatcher: halfMaskedEqual{ + mask: 0x89abcdef, + value: 0x89abcdef, + }, + }, + av, + }, + }, { name: "Common value matchers in PerArg are extracted", rule: Or{ - PerArg{EqualTo(0xA1), EqualTo(0xB1), EqualTo(0xC1), EqualTo(0xD0)}, - PerArg{EqualTo(0xA2), EqualTo(0xB1), EqualTo(0xC1), EqualTo(0xD0)}, - PerArg{EqualTo(0xA1), EqualTo(0xB2), EqualTo(0xC2), EqualTo(0xD0)}, - PerArg{EqualTo(0xA2), EqualTo(0xB2), EqualTo(0xC2), EqualTo(0xD0)}, - PerArg{EqualTo(0xA1), EqualTo(0xB3), EqualTo(0xC3), EqualTo(0xD0)}, - PerArg{EqualTo(0xA2), EqualTo(0xB3), EqualTo(0xC3), EqualTo(0xD0)}, + PerArg{EqualTo(a1), EqualTo(b1), EqualTo(c1), EqualTo(d0)}, + PerArg{EqualTo(a2), EqualTo(b1), EqualTo(c1), EqualTo(d0)}, + PerArg{EqualTo(a1), EqualTo(b2), EqualTo(c2), EqualTo(d0)}, + PerArg{EqualTo(a2), EqualTo(b2), EqualTo(c2), EqualTo(d0)}, + PerArg{EqualTo(a1), EqualTo(b3), EqualTo(c3), EqualTo(d0)}, + PerArg{EqualTo(a2), EqualTo(b3), EqualTo(c3), EqualTo(d0)}, }, want: And{ Or{ - PerArg{EqualTo(0xA1), av, av, av, av, av, av}, - PerArg{EqualTo(0xA2), av, av, av, av, av, av}, + PerArg{s(EqualTo(a1)), av, av, av, av, av, av}, + PerArg{s(EqualTo(a2)), av, av, av, av, av, av}, }, - PerArg{av, av, av, EqualTo(0xD0), av, av, av}, + PerArg{av, av, av, s(EqualTo(d0)), av, av, av}, Or{ - PerArg{av, EqualTo(0xB1), EqualTo(0xC1), av, av, av, av}, - PerArg{av, EqualTo(0xB2), EqualTo(0xC2), av, av, av, av}, - PerArg{av, EqualTo(0xB3), EqualTo(0xC3), av, av, av, av}, + PerArg{av, s(EqualTo(b1)), s(EqualTo(c1)), av, av, av, av}, + PerArg{av, s(EqualTo(b2)), s(EqualTo(c2)), av, av, av, av}, + PerArg{av, s(EqualTo(b3)), s(EqualTo(c3)), av, av, av, av}, }, }, }, @@ -1393,7 +1481,7 @@ func TestOptimizeSyscallRule(t *testing.T) { if len(test.optimizers) == 0 { got = optimizeSyscallRule(test.rule) } else { - got = optimizeSyscallRuleFuncs(test.rule, test.optimizers) + got = (&optimizationRun{funcs: test.optimizers}).optimize(test.rule) } if !reflect.DeepEqual(got, test.want) { t.Errorf("got rule:\n%v\nwant rule:\n%v\n", got, test.want)