dashboard/app: infer bug subsystems from crashes

After each saved crash, invoke the new pkg/subsystem machinery to infer
the subsystem list. Use 5 crashes with biggest priority to base the
inference on.

10 files changed, 161 insertions, 61 deletions

diff --git a/dashboard/app/api.go b/dashboard/app/api.go
index 3108551df..ee39deaa3 100644
--- a/dashboard/app/api.go
+++ b/dashboard/app/api.go
@@ -23,7 +23,7 @@ import (
 	"github.com/google/syzkaller/pkg/auth"
 	"github.com/google/syzkaller/pkg/email"
 	"github.com/google/syzkaller/pkg/hash"
-	subsystem "github.com/google/syzkaller/pkg/subsystem/legacy"
+	"github.com/google/syzkaller/pkg/subsystem"
 	"github.com/google/syzkaller/sys/targets"
 	"golang.org/x/net/context"
 	"google.golang.org/appengine/v2"
@@ -751,21 +751,31 @@ 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(c, 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)
 	}
+
+	newSubsystems := []*subsystem.Subsystem{}
+	// Recalculate subsystems on the first saved crash and on the first saved repro.
+	calculateSubsystems := save && (bug.NumCrashes == 0 ||
+		bug.ReproLevel == ReproLevelNone && reproLevel != ReproLevelNone)
+	if calculateSubsystems {
+		newSubsystems, err = inferSubsystems(c, bug, bugKey)
+		if err != nil {
+			log.Errorf(c, "%q: failed to extract subsystems: %s", bug.Title, err)
+			return nil, err
+		}
+	}
+
 	tx := func(c context.Context) error {
 		bug = new(Bug)
 		if err := db.Get(c, bugKey, bug); err != nil {
@@ -786,8 +796,8 @@ 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})
+		if calculateSubsystems {
+			bug.SetSubsystems(newSubsystems)
 		}
 		bug.increaseCrashStats(now)
 		bug.HappenedOn = mergeString(bug.HappenedOn, build.Manager)
@@ -821,28 +831,6 @@ func parseCrashAssets(c context.Context, req *dashapi.Crash) ([]Asset, error) {
 	return assets, nil
 }
 
-const overrideSubsystemsKey = "set_subsystems"
-
-func detectCrashSubsystems(c context.Context, req *dashapi.Crash, build *Build) []string {
-	val, ok := c.Value(overrideSubsystemsKey).([]string)
-	if ok {
-		return val
-	}
-	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/app_test.go b/dashboard/app/app_test.go
index c14be5d1d..4cc121c32 100644
--- a/dashboard/app/app_test.go
+++ b/dashboard/app/app_test.go
@@ -15,7 +15,8 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/syzkaller/dashboard/dashapi"
 	"github.com/google/syzkaller/pkg/auth"
-	subsystem "github.com/google/syzkaller/pkg/subsystem/legacy"
+	"github.com/google/syzkaller/pkg/subsystem"
+	_ "github.com/google/syzkaller/pkg/subsystem/lists"
 	"github.com/google/syzkaller/sys/targets"
 	"google.golang.org/appengine/v2/user"
 )
@@ -106,6 +107,22 @@ var testConfig = &GlobalConfig{
 					},
 				},
 			},
+			Subsystems: SubsystemsConfig{
+				Service: subsystem.MustMakeService([]*subsystem.Subsystem{
+					{
+						Name:        "subsystemA",
+						PathRules:   []subsystem.PathRule{{IncludeRegexp: `a\.c`}},
+						Lists:       []string{"subsystemA@list.com"},
+						Maintainers: []string{"subsystemA@person.com"},
+					},
+					{
+						Name:        "subsystemB",
+						PathRules:   []subsystem.PathRule{{IncludeRegexp: `b\.c`}},
+						Lists:       []string{"subsystemB@list.com"},
+						Maintainers: []string{"subsystemB@person.com"},
+					},
+				}),
+			},
 		},
 		"test2": {
 			AccessLevel:      AccessAdmin,
@@ -358,7 +375,7 @@ var testConfig = &GlobalConfig{
 				},
 			},
 			Subsystems: SubsystemsConfig{
-				SubsystemCc: subsystem.LinuxGetMaintainers,
+				Service: subsystem.ListService("linux"),
 			},
 		},
 		"test-decommission": {
diff --git a/dashboard/app/config.go b/dashboard/app/config.go
index c2ea93fe4..2bc069706 100644
--- a/dashboard/app/config.go
+++ b/dashboard/app/config.go
@@ -14,6 +14,7 @@ import (
 	"github.com/google/syzkaller/dashboard/dashapi"
 	"github.com/google/syzkaller/pkg/auth"
 	"github.com/google/syzkaller/pkg/email"
+	"github.com/google/syzkaller/pkg/subsystem"
 	"github.com/google/syzkaller/pkg/vcs"
 )
 
@@ -104,11 +105,12 @@ type Config struct {
 	Subsystems SubsystemsConfig
 }
 
-// SubsystemsConfig describes how to generate the list of kernel subsystems and the rules of
-// subsystem inference / bug reporting.
+// SubsystemsConfig describes where to take the list of subsystems and how to infer them.
 type SubsystemsConfig struct {
-	// IMPORTANT: this interface is experimental and will likely change in the future.
-	SubsystemCc func(name string) []string
+	// If Service is set, dashboard will use it to infer and recalculate subsystems.
+	Service *subsystem.Service
+	// If all existing subsystem labels must be recalculated, increase this integer.
+	Revision int64
 }
 
 // ObsoletingConfig describes how bugs without reproducer should be obsoleted.
@@ -348,9 +350,6 @@ 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 524cc6171..3df9c0aca 100644
--- a/dashboard/app/entities.go
+++ b/dashboard/app/entities.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/google/syzkaller/dashboard/dashapi"
 	"github.com/google/syzkaller/pkg/hash"
+	"github.com/google/syzkaller/pkg/subsystem"
 	"golang.org/x/net/context"
 	db "google.golang.org/appengine/v2/datastore"
 )
@@ -129,13 +130,11 @@ type BugSubsystem struct {
 	Name string
 }
 
-func (bug *Bug) addSubsystem(subsystem BugSubsystem) {
-	for _, item := range bug.Tags.Subsystems {
-		if item.Name == subsystem.Name {
-			return
-		}
+func (bug *Bug) SetSubsystems(list []*subsystem.Subsystem) {
+	bug.Tags.Subsystems = nil
+	for _, item := range list {
+		bug.Tags.Subsystems = append(bug.Tags.Subsystems, BugSubsystem{Name: item.Name})
 	}
-	bug.Tags.Subsystems = append(bug.Tags.Subsystems, subsystem)
 }
 
 func (bug *Bug) hasSubsystem(name string) bool {
diff --git a/dashboard/app/linux_reporting.go b/dashboard/app/linux_reporting.go
index c31ef65f4..98220194b 100644
--- a/dashboard/app/linux_reporting.go
+++ b/dashboard/app/linux_reporting.go
@@ -9,7 +9,8 @@ package main
 // canBeVfsBug determines whether a bug could belong to the VFS subsystem itself.
 func canBeVfsBug(bug *Bug) bool {
 	for _, subsystem := range bug.Tags.Subsystems {
-		if subsystem.Name == "vfs" {
+		// The "vfs" one is left for compatibility with the older matching code.
+		if subsystem.Name == "vfs" || subsystem.Name == "fs" {
 			return true
 		}
 	}
diff --git a/dashboard/app/linux_reporting_test.go b/dashboard/app/linux_reporting_test.go
index 1d4e82ac2..004150361 100644
--- a/dashboard/app/linux_reporting_test.go
+++ b/dashboard/app/linux_reporting_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/google/syzkaller/dashboard/dashapi"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestFsSubsystemFlow(t *testing.T) {
@@ -33,7 +34,7 @@ func TestFsSubsystemFlow(t *testing.T) {
 
 	// We skip the first stage and report the bug right away.
 	reply := c.pollEmailBug()
-	c.expectEQ(reply.Subject, "[syzbot] WARNING: abcd")
+	c.expectEQ(reply.Subject, "[syzbot] [kernel?] WARNING: abcd")
 
 	// B. Send a non-vfs bug without a reproducer.
 	// -----------------------------------------
@@ -47,9 +48,10 @@ func TestFsSubsystemFlow(t *testing.T) {
 
 	reply = c.pollEmailBug()
 	// The subsystem should have been taken from the guilty path.
-	c.expectEQ(reply.Subject, "[syzbot] [nilfs2?] WARNING in nilfs_dat_commit_end")
-	c.expectEQ(reply.To, []string{
+	c.expectEQ(reply.Subject, "[syzbot] [nilfs?] WARNING in nilfs_dat_commit_end")
+	assert.ElementsMatch(t, reply.To, []string{
 		"konishi.ryusuke@gmail.com",
+		"linux-fsdevel@vger.kernel.org",
 		"linux-kernel@vger.kernel.org",
 		"linux-nilfs@vger.kernel.org",
 		"maintainer@kernel.org",
@@ -91,11 +93,16 @@ renameat2(r0, &(0x7f00000004c0)='./file0\x00', r0, &(0x7f0000000500)='./bus/file
 	client.updateBug(vfsBug.ID, dashapi.BugStatusUpstream, "")
 	// .. and poll the email.
 	reply = c.pollEmailBug()
-	c.expectEQ(reply.Subject, "[syzbot] [vfs?] [ntfs3?] WARNING in do_mkdirat")
+	c.expectEQ(reply.Subject, "[syzbot] [ntfs3?] WARNING in do_mkdirat")
 	// Make sure ntfs3 maintainers are in the recipients.
-	c.expectEQ(reply.To, []string{"almaz.alexandrovich@paragon-software.com",
-		"linux-kernel@vger.kernel.org", "maintainer@kernel.org",
-		"ntfs3@lists.linux.dev", "test@syzkaller.com"})
+	assert.ElementsMatch(t, reply.To, []string{
+		"almaz.alexandrovich@paragon-software.com",
+		"linux-fsdevel@vger.kernel.org",
+		"linux-kernel@vger.kernel.org",
+		"maintainer@kernel.org",
+		"ntfs3@lists.linux.dev",
+		"test@syzkaller.com",
+	})
 }
 
 func TestVfsSubsystemFlow(t *testing.T) {
@@ -143,5 +150,5 @@ renameat2(r0, &(0x7f00000004c0)='./file0\x00', r0, &(0x7f0000000500)='./bus/file
 	client.updateBug(vfsBug.ID, dashapi.BugStatusUpstream, "")
 	// .. and poll the email.
 	reply := c.pollEmailBug()
-	c.expectEQ(reply.Subject, "[syzbot] [vfs?] WARNING in do_mkdirat2")
+	c.expectEQ(reply.Subject, "[syzbot] [fs?] WARNING in do_mkdirat2")
 }
diff --git a/dashboard/app/main_test.go b/dashboard/app/main_test.go
index 84ee49d70..88cb0dcf5 100644
--- a/dashboard/app/main_test.go
+++ b/dashboard/app/main_test.go
@@ -77,24 +77,27 @@ func TestOnlyManagerFilter(t *testing.T) {
 	}
 }
 
+const (
+	subsystemA = "subsystemA"
+	subsystemB = "subsystemB"
+)
+
 func TestSubsystemFilterMain(t *testing.T) {
 	c := NewCtx(t)
 	defer c.Close()
 
-	subsystemA, subsystemB := "subsystemA", "subsystemB"
-
 	client := c.client
 	build := testBuild(1)
 	client.UploadBuild(build)
 
 	crash1 := testCrash(build, 1)
 	crash1.Title = "first bug"
-	c.contextVars[overrideSubsystemsKey] = []string{subsystemA}
+	crash1.GuiltyFiles = []string{"a.c"}
 	client.ReportCrash(crash1)
 
 	crash2 := testCrash(build, 2)
-	c.contextVars[overrideSubsystemsKey] = []string{subsystemB}
 	crash2.Title = "second bug"
+	crash2.GuiltyFiles = []string{"b.c"}
 	client.ReportCrash(crash2)
 
 	client.pollBugs(2)
@@ -123,20 +126,18 @@ func TestSubsystemFilterTerminal(t *testing.T) {
 	c := NewCtx(t)
 	defer c.Close()
 
-	subsystemA, subsystemB := "subsystemA", "subsystemB"
-
 	client := c.client
 	build := testBuild(1)
 	client.UploadBuild(build)
 
 	crash1 := testCrash(build, 1)
 	crash1.Title = "first bug"
-	c.contextVars[overrideSubsystemsKey] = []string{subsystemA}
+	crash1.GuiltyFiles = []string{"a.c"}
 	client.ReportCrash(crash1)
 
 	crash2 := testCrash(build, 2)
-	c.contextVars[overrideSubsystemsKey] = []string{subsystemB}
 	crash2.Title = "second bug"
+	crash2.GuiltyFiles = []string{"b.c"}
 	client.ReportCrash(crash2)
 
 	// Invalidate all these bugs.
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index 255296216..c96ee6216 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -559,7 +559,7 @@ func fillBugReport(c context.Context, rep *dashapi.BugReport, bug *Bug, bugRepor
 	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))
+			subsystemMaintainers(c, rep.Namespace, item.Name))
 	}
 	for _, addr := range bug.UNCC {
 		rep.CC = email.RemoveFromEmailList(rep.CC, addr)
diff --git a/dashboard/app/subsystem.go b/dashboard/app/subsystem.go
new file mode 100644
index 000000000..d5275ec79
--- /dev/null
+++ b/dashboard/app/subsystem.go
@@ -0,0 +1,64 @@
+// Copyright 2023 syzkaller project authors. All rights reserved.
+// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
+
+package main
+
+import (
+	"fmt"
+
+	"github.com/google/syzkaller/pkg/subsystem"
+	"golang.org/x/net/context"
+	db "google.golang.org/appengine/v2/datastore"
+)
+
+const (
+	// We load the top crashesForInference crashes to determine the bug subsystem(s).
+	crashesForInference = 5
+)
+
+// inferSubsystems determines the best yet possible estimate of the bug's subsystems.
+func inferSubsystems(c context.Context, bug *Bug, bugKey *db.Key) ([]*subsystem.Subsystem, error) {
+	service := getSubsystemService(c, bug.Namespace)
+	if service == nil {
+		// There's nothing we can do.
+		return nil, nil
+	}
+	dbCrashes, dbCrashKeys, err := queryCrashesForBug(c, bugKey, crashesForInference)
+	if err != nil {
+		return nil, err
+	}
+	crashes := []*subsystem.Crash{}
+	for i, dbCrash := range dbCrashes {
+		crash := &subsystem.Crash{}
+		if len(dbCrash.ReportElements.GuiltyFiles) > 0 {
+			// For now we anyway only store one.
+			crash.GuiltyPath = dbCrash.ReportElements.GuiltyFiles[0]
+		}
+		if dbCrash.ReproSyz != 0 {
+			crash.SyzRepro, _, err = getText(c, textReproSyz, dbCrash.ReproSyz)
+			if err != nil {
+				return nil, fmt.Errorf("failed to load syz repro for %s: %w",
+					dbCrashKeys[i], err)
+			}
+		}
+		crashes = append(crashes, crash)
+	}
+	return service.Extract(crashes), nil
+}
+
+// subsystemMaintainers queries the list of emails to send the bug to.
+func subsystemMaintainers(c context.Context, ns, subsystemName string) []string {
+	service := getSubsystemService(c, ns)
+	if service == nil {
+		return nil
+	}
+	item := service.ByName(subsystemName)
+	if item == nil {
+		return nil
+	}
+	return item.Emails()
+}
+
+func getSubsystemService(c context.Context, ns string) *subsystem.Service {
+	return config.Namespaces[ns].Subsystems.Service
+}
diff --git a/dashboard/app/subsystem_test.go b/dashboard/app/subsystem_test.go
new file mode 100644
index 000000000..e4f82f220
--- /dev/null
+++ b/dashboard/app/subsystem_test.go
@@ -0,0 +1,24 @@
+// Copyright 2023 syzkaller project authors. All rights reserved.
+// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
+
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSubsytemMaintainers(t *testing.T) {
+	c := NewCtx(t)
+	defer c.Close()
+
+	// This also indirectly tests getSubsystemService.
+	assert.ElementsMatch(t,
+		subsystemMaintainers(c.ctx, "test1", "subsystemA"),
+		[]string{
+			"subsystemA@list.com", "subsystemA@person.com",
+		},
+	)
+	assert.ElementsMatch(t, subsystemMaintainers(c.ctx, "test1", "does-not-exist"), []string{})
+}