diff --git a/pkg/seccomp/seccomp.go b/pkg/seccomp/seccomp.go index 60c1047ad..8b337f8ee 100644 --- a/pkg/seccomp/seccomp.go +++ b/pkg/seccomp/seccomp.go @@ -309,6 +309,10 @@ type ProgramOptions struct { // syscall structure input doesn't match the one the program expects. BadArchAction linux.BPFAction + // Optimize specifies whether optimizations should be applied to the + // syscall rules and generated BPF bytecode. + Optimize bool + // HotSyscalls is the set of syscall numbers that are the hottest, // where "hotness" refers to frequency (regardless of the amount of // computation that the kernel will do handling them, and regardless of @@ -328,6 +332,7 @@ func DefaultProgramOptions() ProgramOptions { return ProgramOptions{ DefaultAction: action, BadArchAction: action, + Optimize: true, } } @@ -354,13 +359,11 @@ type BuildStats struct { // SyscallRules. The single generated program covers all provided RuleSets. func BuildProgram(rules []RuleSet, options ProgramOptions) ([]bpf.Instruction, BuildStats, error) { start := time.Now() - - // Make a copy of the syscall rules and optimize them. - ors, err := orderRuleSets(rules, options) + // Make a copy of the syscall rules and maybe optimize them. + ors, ruleOptimizeDuration, err := orderRuleSets(rules, options) if err != nil { return nil, BuildStats{}, err } - ruleOptimizeDuration := time.Since(start) possibleActions := make(map[linux.BPFAction]struct{}) for _, ruleSet := range rules { @@ -398,10 +401,14 @@ func BuildProgram(rules []RuleSet, options ProgramOptions) ([]bpf.Instruction, B } beforeOpt := len(insns) buildDuration := time.Since(start) - ruleOptimizeDuration - insns = bpf.Optimize(insns) - bpfOptimizeDuration := time.Since(start) - ruleOptimizeDuration - buildDuration - afterOpt := len(insns) - log.Debugf("Seccomp program optimized from %d to %d instructions; took %v to build and %v to optimize", beforeOpt, afterOpt, buildDuration, bpfOptimizeDuration) + var bpfOptimizeDuration time.Duration + afterOpt := beforeOpt + if options.Optimize { + insns = bpf.Optimize(insns) + bpfOptimizeDuration = time.Since(start) - buildDuration - ruleOptimizeDuration + afterOpt = len(insns) + log.Debugf("Seccomp program optimized from %d to %d instructions; took %v to build and %v to optimize", beforeOpt, afterOpt, buildDuration, bpfOptimizeDuration) + } return insns, BuildStats{ SizeBeforeOptimizations: beforeOpt, SizeAfterOptimizations: afterOpt, @@ -530,14 +537,16 @@ type orderedRuleSets struct { } // orderRuleSets converts a set of `RuleSet`s into an `orderedRuleSets`. -func orderRuleSets(rules []RuleSet, options ProgramOptions) (orderedRuleSets, error) { +// It orders the rulesets, along with the time to optimize the +// rules (if any). +func orderRuleSets(rules []RuleSet, options ProgramOptions) (orderedRuleSets, time.Duration, error) { // Do a pass to determine if vsyscall is consistent across syscall numbers. vsyscallBySysno := make(map[uintptr]bool) for _, rs := range rules { for sysno := range rs.Rules.rules { if prevVsyscall, ok := vsyscallBySysno[sysno]; ok { if prevVsyscall != rs.Vsyscall { - return orderedRuleSets{}, fmt.Errorf("syscall %d has conflicting vsyscall checking rules", sysno) + return orderedRuleSets{}, 0, fmt.Errorf("syscall %d has conflicting vsyscall checking rules", sysno) } } else { vsyscallBySysno[sysno] = rs.Vsyscall @@ -573,11 +582,16 @@ func orderRuleSets(rules []RuleSet, options ProgramOptions) (orderedRuleSets, er } // Optimize all rules. - for _, ruleActions := range allSyscallRuleActions { - for i, ra := range ruleActions { - ra.rule = optimizeSyscallRule(ra.rule) - ruleActions[i] = ra + var optimizeDuration time.Duration + if options.Optimize { + optimizeStart := time.Now() + for _, ruleActions := range allSyscallRuleActions { + for i, ra := range ruleActions { + ra.rule = optimizeSyscallRule(ra.rule) + ruleActions[i] = ra + } } + optimizeDuration = time.Since(optimizeStart) } // Do a pass that checks which syscall numbers are trivial. @@ -648,7 +662,7 @@ func orderRuleSets(rules []RuleSet, options ProgramOptions) (orderedRuleSets, er ors.log(log.Debugf) } - return ors, nil + return ors, optimizeDuration, nil } // log logs the set of seccomp rules to the given logger. diff --git a/pkg/seccomp/seccomp_test.go b/pkg/seccomp/seccomp_test.go index 79f7e8324..c51c66579 100644 --- a/pkg/seccomp/seccomp_test.go +++ b/pkg/seccomp/seccomp_test.go @@ -1383,7 +1383,7 @@ func TestOrderRuleSets(t *testing.T) { for _, test := range []struct { name string ruleSets []RuleSet - options ProgramOptions + options ProgramOptions // Optimizations are always enabled regardless of this want orderedRuleSets wantErr bool }{ @@ -1896,7 +1896,8 @@ func TestOrderRuleSets(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - got, gotErr := orderRuleSets(test.ruleSets, test.options) + test.options.Optimize = true + got, _, gotErr := orderRuleSets(test.ruleSets, test.options) if (gotErr != nil) != test.wantErr { t.Errorf("got error: %v, want error: %v", gotErr, test.wantErr) } diff --git a/runsc/boot/filter/BUILD b/runsc/boot/filter/BUILD index 8df9f76e3..194e20493 100644 --- a/runsc/boot/filter/BUILD +++ b/runsc/boot/filter/BUILD @@ -64,3 +64,15 @@ secbench_test( "@org_golang_x_sys//unix:go_default_library", ], ) + +go_test( + name = "filter_fuzz_test", + srcs = ["filter_fuzz_test.go"], + deps = [ + ":filter", + "//pkg/abi/linux", + "//pkg/seccomp", + "//pkg/sentry/platform/systrap", + "//test/secfuzz", + ], +) diff --git a/runsc/boot/filter/filter_fuzz_test.go b/runsc/boot/filter/filter_fuzz_test.go new file mode 100644 index 000000000..b436c99cb --- /dev/null +++ b/runsc/boot/filter/filter_fuzz_test.go @@ -0,0 +1,79 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package filter_fuzz_test + +import ( + "testing" + + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/seccomp" + "gvisor.dev/gvisor/pkg/sentry/platform/systrap" + "gvisor.dev/gvisor/runsc/boot/filter" + "gvisor.dev/gvisor/test/secfuzz" +) + +// FuzzFilterOptimizationsResultInConsistentProgram tests that optimizations +// do not affect the behavior of the generated seccomp-bpf program. +func FuzzFilterOptimizationsResultInConsistentProgram(f *testing.F) { + filterOpts := filter.Options{ + Platform: &systrap.Systrap{}, + } + rules, denyRules := filter.Rules(filterOpts) + ruleSets := []seccomp.RuleSet{ + { + Rules: denyRules, + Action: linux.SECCOMP_RET_ERRNO, + }, + { + Rules: rules, + Action: linux.SECCOMP_RET_ALLOW, + }, + } + unoptimizedOpts := filter.SeccompOptions(filterOpts) + unoptimizedOpts.Optimize = false + unoptimized, _, err := seccomp.BuildProgram(ruleSets, unoptimizedOpts) + if err != nil { + f.Fatalf("failed to build unoptimized program: %v", err) + } + fuzzeeUnoptimized := secfuzz.Fuzzee{ + Name: "unoptimized", + Instructions: unoptimized, + + // We cannot enforce full coverage on the unoptimized program, + // because some of its checks are impossible to meet. + // For example, it ends up checking things like + // "if (A & 0) == 0" when checking both 32-bit halves of a + // "masked equal" check, and the "false" branch of that can + // never be covered. + EnforceFullCoverage: false, + } + optimizedOpts := filter.SeccompOptions(filterOpts) + optimizedOpts.Optimize = true + optimized, _, err := seccomp.BuildProgram(ruleSets, optimizedOpts) + if err != nil { + f.Fatalf("failed to build optimized program: %v", err) + } + fuzzeeOptimized := secfuzz.Fuzzee{ + Name: "optimized", + Instructions: optimized, + EnforceFullCoverage: true, + } + df, err := secfuzz.NewDiffFuzzer(f, &fuzzeeUnoptimized, &fuzzeeOptimized) + if err != nil { + f.Fatalf("failed to create diff fuzzer: %v", err) + } + df.DeriveCorpusFromRuleSets(ruleSets) + df.Fuzz() +}