From f38211dffe4374a9e09d966b021c00da9d1300c5 Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Wed, 14 Dec 2022 19:09:08 +0100 Subject: dashboard: extract subsystems for saved crashes Invoke the extractor code at pkg/subsystem and store the result into the Bug entity. Augment the Maintainers content by per-subsystem emails. --- dashboard/app/api.go | 25 ++++++++++++++++++++++++- dashboard/app/config.go | 12 ++++++++++++ dashboard/app/entities.go | 9 +++++++++ dashboard/app/reporting.go | 8 +++++--- pkg/subsystem/extract.go | 31 ++++++++++++++++++++++--------- pkg/subsystem/extract_test.go | 5 +++-- 6 files changed, 75 insertions(+), 15 deletions(-) diff --git a/dashboard/app/api.go b/dashboard/app/api.go index 253a6be96..f7f139f86 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -23,6 +23,7 @@ import ( "github.com/google/syzkaller/pkg/auth" "github.com/google/syzkaller/pkg/email" "github.com/google/syzkaller/pkg/hash" + "github.com/google/syzkaller/pkg/subsystem" "github.com/google/syzkaller/sys/targets" "golang.org/x/net/context" "google.golang.org/appengine/v2" @@ -700,6 +701,7 @@ func apiReportCrash(c context.Context, ns string, r *http.Request, payload []byt return resp, nil } +// nolint: gocyclo func reportCrash(c context.Context, build *Build, req *dashapi.Crash) (*Bug, error) { assets, err := parseCrashAssets(c, req) if err != nil { @@ -736,19 +738,21 @@ func reportCrash(c context.Context, build *Build, req *dashapi.Crash) (*Bug, err } else if len(req.ReproSyz) != 0 { reproLevel = ReproLevelSyz } + newSubsystems := []string{} save := reproLevel != ReproLevelNone || bug.NumCrashes < int64(maxCrashes()) || now.Sub(bug.LastSavedCrash) > time.Hour || bug.NumCrashes%20 == 0 || !stringInList(bug.MergedTitles, req.Title) if save { + newSubsystems = detectCrashSubsystems(req, build) + log.Infof(c, "determined subsystems: %q", newSubsystems) if err := saveCrash(c, ns, req, bug, bugKey, build, assets); err != nil { return nil, err } } else { log.Infof(c, "not saving crash for %q", bug.Title) } - tx := func(c context.Context) error { bug = new(Bug) if err := db.Get(c, bugKey, bug); err != nil { @@ -769,6 +773,9 @@ func reportCrash(c context.Context, build *Build, req *dashapi.Crash) (*Bug, err if len(req.Report) != 0 { bug.HasReport = true } + for _, name := range newSubsystems { + bug.addSubsystem(BugSubsystem{name}) + } bug.increaseCrashStats(now) bug.HappenedOn = mergeString(bug.HappenedOn, build.Manager) // Migration of older entities (for new bugs Title is always in MergedTitles). @@ -801,6 +808,22 @@ func parseCrashAssets(c context.Context, req *dashapi.Crash) ([]Asset, error) { return assets, nil } +func detectCrashSubsystems(req *dashapi.Crash, build *Build) []string { + if build.OS == targets.Linux { + extractor := subsystem.MakeLinuxSubsystemExtractor() + return extractor.Extract(&subsystem.Crash{ + GuiltyFiles: req.GuiltyFiles, + SyzRepro: string(req.ReproSyz), + }) + } + return nil +} + +func subsystemMaintainers(ns, subsystemName string) []string { + nsConfig := config.Namespaces[ns] + return nsConfig.Subsystems.SubsystemCc(subsystemName) +} + func (crash *Crash) UpdateReportingPriority(build *Build, bug *Bug) { prio := int64(kernelRepoInfo(build).ReportingPriority) * 1e6 divReproPrio := int64(1) diff --git a/dashboard/app/config.go b/dashboard/app/config.go index 52a98beb4..c2ea93fe4 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -100,6 +100,15 @@ type Config struct { Repos []KernelRepo // If not nil, bugs in this namespace will be exported to the specified Kcidb. Kcidb *KcidbConfig + // Subsystems config. + Subsystems SubsystemsConfig +} + +// SubsystemsConfig describes how to generate the list of kernel subsystems and the rules of +// subsystem inference / bug reporting. +type SubsystemsConfig struct { + // IMPORTANT: this interface is experimental and will likely change in the future. + SubsystemCc func(name string) []string } // ObsoletingConfig describes how bugs without reproducer should be obsoleted. @@ -339,6 +348,9 @@ func checkNamespace(ns string, cfg *Config, namespaces, clientNames map[string]b if cfg.Kcidb != nil { checkKcidb(ns, cfg.Kcidb) } + if cfg.Subsystems.SubsystemCc == nil { + cfg.Subsystems.SubsystemCc = func(string) []string { return nil } + } checkKernelRepos(ns, cfg) checkNamespaceReporting(ns, cfg) } diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go index 2cbd891eb..97d4c7012 100644 --- a/dashboard/app/entities.go +++ b/dashboard/app/entities.go @@ -129,6 +129,15 @@ type BugSubsystem struct { Name string } +func (bug *Bug) addSubsystem(subsystem BugSubsystem) { + for _, item := range bug.Tags.Subsystems { + if item.Name == subsystem.Name { + return + } + } + bug.Tags.Subsystems = append(bug.Tags.Subsystems, subsystem) +} + func (bug *Bug) Load(ps []db.Property) error { if err := db.LoadStruct(bug, ps); err != nil { return err diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go index aa615f47c..97727532a 100644 --- a/dashboard/app/reporting.go +++ b/dashboard/app/reporting.go @@ -591,13 +591,15 @@ func fillBugReport(c context.Context, rep *dashapi.BugReport, bug *Bug, bugRepor rep.KernelConfigLink = externalLink(c, textKernelConfig, build.KernelConfig) rep.SyzkallerCommit = build.SyzkallerCommit rep.NoRepro = build.Type == BuildFailed + for _, item := range bug.Tags.Subsystems { + rep.Subsystems = append(rep.Subsystems, dashapi.BugSubsystem{Name: item.Name}) + rep.Maintainers = email.MergeEmailLists(rep.Maintainers, + subsystemMaintainers(rep.Namespace, item.Name)) + } for _, addr := range bug.UNCC { rep.CC = email.RemoveFromEmailList(rep.CC, addr) rep.Maintainers = email.RemoveFromEmailList(rep.Maintainers, addr) } - for _, item := range bug.Tags.Subsystems { - rep.Subsystems = append(rep.Subsystems, dashapi.BugSubsystem{Name: item.Name}) - } return nil } diff --git a/pkg/subsystem/extract.go b/pkg/subsystem/extract.go index caf80636e..181c15835 100644 --- a/pkg/subsystem/extract.go +++ b/pkg/subsystem/extract.go @@ -7,11 +7,11 @@ import ( "regexp" "github.com/google/syzkaller/prog" - "github.com/google/syzkaller/sys/targets" ) type SubsystemExtractor struct { - CallToSubsystems func(call string) []string + pathToSubsystems func(path string) []string + callToSubsystems func(call string) []string } // Crash contains the subset of Crash fields relevant for subsystem extraction. @@ -21,21 +21,26 @@ type Crash struct { SyzRepro string } +func MakeLinuxSubsystemExtractor() *SubsystemExtractor { + return &SubsystemExtractor{ + pathToSubsystems: linuxPathToSubsystems, + } +} + func (se *SubsystemExtractor) Extract(crash *Crash) []string { retMap := map[string]bool{} // Currently we only have the dumbest possible implementation of subsystem detection. - if crash.OS == targets.Linux { - for _, guiltyPath := range crash.GuiltyFiles { - if vfsPathRegexp.MatchString(guiltyPath) { - retMap["vfs"] = true - break + if se.pathToSubsystems != nil { + for _, path := range crash.GuiltyFiles { + for _, value := range se.pathToSubsystems(path) { + retMap[value] = true } } } - if se.CallToSubsystems != nil { + if se.callToSubsystems != nil { callSet, _, _ := prog.CallSet([]byte(crash.SyzRepro)) for call := range callSet { - for _, subsystem := range se.CallToSubsystems(call) { + for _, subsystem := range se.callToSubsystems(call) { retMap[subsystem] = true } } @@ -47,6 +52,14 @@ func (se *SubsystemExtractor) Extract(crash *Crash) []string { return retSlice } +func linuxPathToSubsystems(path string) []string { + ret := []string{} + if vfsPathRegexp.MatchString(path) { + ret = append(ret, "vfs") + } + return ret +} + var ( vfsPathRegexp = regexp.MustCompile(`^fs/[^/]+\.c`) ) diff --git a/pkg/subsystem/extract_test.go b/pkg/subsystem/extract_test.go index 936f4fa8e..29512f460 100644 --- a/pkg/subsystem/extract_test.go +++ b/pkg/subsystem/extract_test.go @@ -12,7 +12,7 @@ import ( ) func TestSimpleLinuxExtract(t *testing.T) { - se := &SubsystemExtractor{} + se := MakeLinuxSubsystemExtractor() ret := se.Extract(&Crash{ OS: targets.Linux, @@ -34,7 +34,8 @@ func TestSimpleLinuxExtract(t *testing.T) { func TestProgCallRules(t *testing.T) { se := &SubsystemExtractor{ - CallToSubsystems: func(call string) []string { + pathToSubsystems: linuxPathToSubsystems, + callToSubsystems: func(call string) []string { ret := map[string][]string{ // Intentionally add some that are not present in the test below. "test": {"test"}, -- cgit mrf-deployment