From 4459585c043faace507c685bcd9997da15809aae Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Wed, 8 Dec 2021 17:02:32 +0000 Subject: all: adapt to how mmapping a kcov instance works in Linux It turns out that the current Linux implementation of KCOV does not properly handle multiple mmap invocations on the same instance. The first one succeedes, but the subsequent ones do not actually mmap anything, yet returning no error at all. The ability to mmap that memory multiple times allows us to increase syz-executor performance and it would be a pity to completely lose it (especially given that mmapping kcov works fine on *BSD). In some time a patch will be prepared, but still we will have to support both versions at the same time - the buggy one and the correct one. Detect whether the bug is present by writing a value at the pointer returned by mmap. If it is present, disable dynamic kcov mmapping and pre-mmap 5 instances in the main() function - it should be enough for all reasonable uses. Otherwise, pre-mmap 3 and let syz-executor mmap them as needed. --- executor/executor.cc | 26 +++++++++++++++--- pkg/host/features.go | 2 ++ pkg/host/features_linux.go | 62 ++++++++++++++++++++++++++++++++---------- pkg/host/host_freebsd.go | 1 + pkg/host/host_netbsd.go | 1 + pkg/host/host_openbsd.go | 1 + pkg/ipc/ipc.go | 1 + pkg/runtest/run.go | 3 ++ syz-fuzzer/fuzzer.go | 3 ++ tools/syz-execprog/execprog.go | 3 ++ 10 files changed, 85 insertions(+), 18 deletions(-) diff --git a/executor/executor.cc b/executor/executor.cc index e4034310d..5f19c079f 100644 --- a/executor/executor.cc +++ b/executor/executor.cc @@ -67,7 +67,6 @@ typedef unsigned char uint8; // Some common_OS.h files know about this constant for RLIMIT_NOFILE. const int kMaxFd = 250; const int kMaxThreads = 32; -const int kPreMmapCoverThreads = 3; // the number of kcov instances to mmap during init const int kInPipeFd = kMaxFd - 1; // remapped from stdin const int kOutPipeFd = kMaxFd - 2; // remapped from stdout const int kCoverFd = kOutPipeFd - kMaxThreads; @@ -76,6 +75,11 @@ const int kMaxArgs = 9; const int kCoverSize = 256 << 10; const int kFailStatus = 67; +// Two approaches of dealing with kcov memory. +const int kCoverOptimizedCount = 12; // the number of kcov instances to be opened inside main() +const int kCoverOptimizedPreMmap = 3; // this many will be mmapped inside main(), others - when needed. +const int kCoverDefaultCount = 5; // otherwise we only init kcov instances inside main() + // Logical error (e.g. invalid input program), use as an assert() alternative. // If such error happens 10+ times in a row, it will be detected as a bug by syz-fuzzer. // syz-fuzzer will fail and syz-manager will create a bug for this. @@ -167,6 +171,7 @@ static bool flag_close_fds; static bool flag_devlink_pci; static bool flag_vhci_injection; static bool flag_wifi; +static bool flag_delay_kcov_mmap; static bool flag_collect_cover; static bool flag_dedup_cover; @@ -469,10 +474,17 @@ int main(int argc, char** argv) receive_execute(); #endif if (flag_coverage) { - for (int i = 0; i < kMaxThreads; i++) { + int create_count = kCoverDefaultCount, mmap_count = create_count; + if (flag_delay_kcov_mmap) { + create_count = kCoverOptimizedCount; + mmap_count = kCoverOptimizedPreMmap; + } + if (create_count > kMaxThreads) + create_count = kMaxThreads; + for (int i = 0; i < create_count; i++) { threads[i].cov.fd = kCoverFd + i; cover_open(&threads[i].cov, false); - if (i < kPreMmapCoverThreads) { + if (i < mmap_count) { // Pre-mmap coverage collection for some threads. This should be enough for almost // all programs, for the remaning few ones coverage will be set up when it's needed. thread_mmap_cover(&threads[i]); @@ -600,6 +612,7 @@ void parse_env_flags(uint64 flags) flag_devlink_pci = flags & (1 << 11); flag_vhci_injection = flags & (1 << 12); flag_wifi = flags & (1 << 13); + flag_delay_kcov_mmap = flags & (1 << 14); } #if SYZ_EXECUTOR_USES_FORK_SERVER @@ -1193,8 +1206,11 @@ void thread_create(thread_t* th, int id, bool need_coverage) th->executing = false; // Lazily set up coverage collection. // It is assumed that actually it's already initialized - with a few rare exceptions. - if (need_coverage) + if (need_coverage) { + if (!th->cov.fd) + fail("out of opened kcov threads"); thread_mmap_cover(th); + } event_init(&th->ready); event_init(&th->done); event_set(&th->done); @@ -1206,6 +1222,8 @@ void thread_mmap_cover(thread_t* th) { if (th->cov.data != NULL) return; + if (!flag_delay_kcov_mmap) + fail("out of mmapped kcov threads"); cover_mmap(&th->cov); cover_protect(&th->cov); } diff --git a/pkg/host/features.go b/pkg/host/features.go index f3f4af30c..d2105b203 100644 --- a/pkg/host/features.go +++ b/pkg/host/features.go @@ -19,6 +19,7 @@ const ( FeatureCoverage = iota FeatureComparisons FeatureExtraCoverage + FeatureDelayKcovMmap FeatureSandboxSetuid FeatureSandboxNamespace FeatureSandboxAndroid @@ -60,6 +61,7 @@ func Check(target *prog.Target) (*Features, error) { FeatureCoverage: {Name: "code coverage", Reason: unsupported}, FeatureComparisons: {Name: "comparison tracing", Reason: unsupported}, FeatureExtraCoverage: {Name: "extra coverage", Reason: unsupported}, + FeatureDelayKcovMmap: {Name: "delay kcov mmap", Reason: unsupported}, FeatureSandboxSetuid: {Name: "setuid sandbox", Reason: unsupported}, FeatureSandboxNamespace: {Name: "namespace sandbox", Reason: unsupported}, FeatureSandboxAndroid: {Name: "Android sandbox", Reason: unsupported}, diff --git a/pkg/host/features_linux.go b/pkg/host/features_linux.go index 5c45bdcc1..de04e7a4d 100644 --- a/pkg/host/features_linux.go +++ b/pkg/host/features_linux.go @@ -7,6 +7,7 @@ import ( "fmt" "regexp" "runtime" + "runtime/debug" "strconv" "syscall" "unsafe" @@ -21,6 +22,7 @@ func init() { checkFeature[FeatureCoverage] = checkCoverage checkFeature[FeatureComparisons] = checkComparisons checkFeature[FeatureExtraCoverage] = checkExtraCoverage + checkFeature[FeatureDelayKcovMmap] = checkDelayKcovMmap checkFeature[FeatureSandboxSetuid] = unconditionallyEnabled checkFeature[FeatureSandboxNamespace] = checkSandboxNamespace checkFeature[FeatureSandboxAndroid] = checkSandboxAndroid @@ -57,6 +59,39 @@ func checkExtraCoverage() (reason string) { return checkCoverageFeature(FeatureExtraCoverage) } +func checkDelayKcovMmap() string { + // The kcov implementation in Linux (currently) does not adequately handle subsequent + // mmap invocations on the same descriptor. When that problem is fixed, we want + // syzkaller to differentiate between distributions as this allows to noticeably speed + // up fuzzing. + return checkCoverageFeature(FeatureDelayKcovMmap) +} + +func kcovTestMmap(fd int, coverSize uintptr) (reason string) { + mem, err := syscall.Mmap(fd, 0, int(coverSize*unsafe.Sizeof(uintptr(0))), + syscall.PROT_READ|syscall.PROT_WRITE, syscall.MAP_SHARED) + if err != nil { + return fmt.Sprintf("KCOV mmap failed: %v", err) + } + defer func() { + if err := syscall.Munmap(mem); err != nil { + reason = fmt.Sprintf("munmap failed: %v", err) + } + }() + // Now the tricky part - mmap may say it has succeeded, but the memory is + // in fact not allocated. + // We try to access it. If go does not panic, everything is fine. + prevValue := debug.SetPanicOnFault(true) + defer debug.SetPanicOnFault(prevValue) + defer func() { + if r := recover(); r != nil { + reason = "mmap returned an invalid pointer" + } + }() + mem[0] = 0 + return "" +} + func checkCoverageFeature(feature int) (reason string) { if reason = checkDebugFS(); reason != "" { return reason @@ -80,16 +115,15 @@ func checkCoverageFeature(feature int) (reason string) { if errno != 0 { return fmt.Sprintf("ioctl(KCOV_INIT_TRACE) failed: %v", errno) } - mem, err := syscall.Mmap(fd, 0, int(coverSize*unsafe.Sizeof(uintptr(0))), - syscall.PROT_READ|syscall.PROT_WRITE, syscall.MAP_SHARED) - if err != nil { - return fmt.Sprintf("KCOV mmap failed: %v", err) + if reason = kcovTestMmap(fd, coverSize); reason != "" { + return reason } - defer func() { - if err := syscall.Munmap(mem); err != nil { - reason = fmt.Sprintf("munmap failed: %v", err) + disableKcov := func() { + _, _, errno = syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), linux.KCOV_DISABLE, 0) + if errno != 0 { + reason = fmt.Sprintf("ioctl(KCOV_DISABLE) failed: %v", errno) } - }() + } switch feature { case FeatureComparisons: _, _, errno = syscall.Syscall(syscall.SYS_IOCTL, @@ -100,6 +134,7 @@ func checkCoverageFeature(feature int) (reason string) { } return fmt.Sprintf("ioctl(KCOV_TRACE_CMP) failed: %v", errno) } + defer disableKcov() case FeatureExtraCoverage: arg := KcovRemoteArg{ TraceMode: uint32(linux.KCOV_TRACE_PC), @@ -115,15 +150,14 @@ func checkCoverageFeature(feature int) (reason string) { } return fmt.Sprintf("ioctl(KCOV_REMOTE_ENABLE) failed: %v", errno) } + defer disableKcov() + case FeatureDelayKcovMmap: + if reason = kcovTestMmap(fd, coverSize); reason != "" { + return reason + } default: panic("unknown feature in checkCoverageFeature") } - defer func() { - _, _, errno = syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), linux.KCOV_DISABLE, 0) - if errno != 0 { - reason = fmt.Sprintf("ioctl(KCOV_DISABLE) failed: %v", errno) - } - }() return "" } diff --git a/pkg/host/host_freebsd.go b/pkg/host/host_freebsd.go index 0f86a1f9c..726429c18 100644 --- a/pkg/host/host_freebsd.go +++ b/pkg/host/host_freebsd.go @@ -14,5 +14,6 @@ func isSupported(c *prog.Syscall, target *prog.Target, sandbox string) (bool, st func init() { checkFeature[FeatureCoverage] = unconditionallyEnabled checkFeature[FeatureComparisons] = unconditionallyEnabled + checkFeature[FeatureDelayKcovMmap] = unconditionallyEnabled checkFeature[FeatureNetInjection] = unconditionallyEnabled } diff --git a/pkg/host/host_netbsd.go b/pkg/host/host_netbsd.go index aa0a6e88a..e4de2d408 100644 --- a/pkg/host/host_netbsd.go +++ b/pkg/host/host_netbsd.go @@ -23,6 +23,7 @@ func init() { checkFeature[FeatureComparisons] = unconditionallyEnabled checkFeature[FeatureUSBEmulation] = checkUSBEmulation checkFeature[FeatureExtraCoverage] = checkUSBEmulation + checkFeature[FeatureDelayKcovMmap] = unconditionallyEnabled checkFeature[FeatureFault] = checkFault } diff --git a/pkg/host/host_openbsd.go b/pkg/host/host_openbsd.go index 23e219115..3099bc771 100644 --- a/pkg/host/host_openbsd.go +++ b/pkg/host/host_openbsd.go @@ -32,6 +32,7 @@ func init() { checkFeature[FeatureCoverage] = unconditionallyEnabled checkFeature[FeatureComparisons] = unconditionallyEnabled checkFeature[FeatureExtraCoverage] = unconditionallyEnabled + checkFeature[FeatureDelayKcovMmap] = unconditionallyEnabled checkFeature[FeatureNetInjection] = unconditionallyEnabled checkFeature[FeatureSandboxSetuid] = unconditionallyEnabled } diff --git a/pkg/ipc/ipc.go b/pkg/ipc/ipc.go index f6bf3f61f..03b28e4ce 100644 --- a/pkg/ipc/ipc.go +++ b/pkg/ipc/ipc.go @@ -42,6 +42,7 @@ const ( FlagEnableDevlinkPCI // setup devlink PCI device FlagEnableVhciInjection // setup and use /dev/vhci for hci packet injection FlagEnableWifi // setup and use mac80211_hwsim for wifi emulation + FlagDelayKcovMmap // manage kcov memory in an optimized way ) // Per-exec flags for ExecOpts.Flags. diff --git a/pkg/runtest/run.go b/pkg/runtest/run.go index 5a071676a..1d3412cb0 100644 --- a/pkg/runtest/run.go +++ b/pkg/runtest/run.go @@ -412,6 +412,9 @@ func (ctx *Context) createSyzTest(p *prog.Prog, sandbox string, threaded, cov bo if ctx.Features[host.FeatureExtraCoverage].Enabled { cfg.Flags |= ipc.FlagExtraCover } + if ctx.Features[host.FeatureDelayKcovMmap].Enabled { + cfg.Flags |= ipc.FlagDelayKcovMmap + } if ctx.Features[host.FeatureNetInjection].Enabled { cfg.Flags |= ipc.FlagEnableTun } diff --git a/syz-fuzzer/fuzzer.go b/syz-fuzzer/fuzzer.go index 0eb4791d3..d8961bfae 100644 --- a/syz-fuzzer/fuzzer.go +++ b/syz-fuzzer/fuzzer.go @@ -108,6 +108,9 @@ func createIPCConfig(features *host.Features, config *ipc.Config) { if features[host.FeatureExtraCoverage].Enabled { config.Flags |= ipc.FlagExtraCover } + if features[host.FeatureDelayKcovMmap].Enabled { + config.Flags |= ipc.FlagDelayKcovMmap + } if features[host.FeatureNetInjection].Enabled { config.Flags |= ipc.FlagEnableTun } diff --git a/tools/syz-execprog/execprog.go b/tools/syz-execprog/execprog.go index 22729ee87..6c69e0ff6 100644 --- a/tools/syz-execprog/execprog.go +++ b/tools/syz-execprog/execprog.go @@ -318,6 +318,9 @@ func createConfig(target *prog.Target, features *host.Features, featuresFlags cs if features[host.FeatureExtraCoverage].Enabled { config.Flags |= ipc.FlagExtraCover } + if features[host.FeatureDelayKcovMmap].Enabled { + config.Flags |= ipc.FlagDelayKcovMmap + } if featuresFlags["tun"].Enabled && features[host.FeatureNetInjection].Enabled { config.Flags |= ipc.FlagEnableTun } -- cgit mrf-deployment