aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAleksandr Nogikh <nogikh@google.com>2022-05-30 13:17:24 +0000
committerAleksandr Nogikh <wp32pw@gmail.com>2022-05-31 14:34:53 +0200
commit3666edfeb55080ebe138d77417fa96fe2555d6bb (patch)
tree6ab82b9954c03306a5262c5692fbb83da34d400a
parentaf70c3a9d26d6637e932facd13b1e55dd96270b5 (diff)
dashboard: never report unreliable bisections
Currently the dashboard does not report unreliable bisections themselves, but there was a way in which they could still leak. The recently detected case is when the bisection was done using a syz repro, no reporting was made, but with a message about a newly found C repro syzbot included the bisection info and Cc'd bisection-sourced emails. Fix that behavior and introduce one new test.
-rw-r--r--dashboard/app/bisect_test.go48
-rw-r--r--dashboard/app/reporting.go15
2 files changed, 58 insertions, 5 deletions
diff --git a/dashboard/app/bisect_test.go b/dashboard/app/bisect_test.go
index 624f8f876..71a86198c 100644
--- a/dashboard/app/bisect_test.go
+++ b/dashboard/app/bisect_test.go
@@ -545,6 +545,54 @@ https://goo.gl/tpsmEJ#testing-patches`,
}
}
+func TestUnreliableBisect(t *testing.T) {
+ c := NewCtx(t)
+ defer c.Close()
+
+ build := testBuild(1)
+ c.client2.UploadBuild(build)
+ // Upload a crash that has only a syz repro.
+ crash := testCrashWithRepro(build, 1)
+ crash.ReproC = nil
+ c.client2.ReportCrash(crash)
+ _ = c.client2.pollEmailBug()
+
+ pollResp := c.client2.pollJobs(build.Manager)
+ jobID := pollResp.ID
+ done := &dashapi.JobDoneReq{
+ ID: jobID,
+ Build: *build,
+ Log: []byte("bisect log"),
+ Flags: dashapi.BisectResultRelease,
+ Commits: []dashapi.Commit{
+ {
+ Hash: "111111111111111111111111",
+ Title: "Linux 4.10",
+ Author: "abcd@kernel.org",
+ AuthorName: "Abcd Efgh",
+ CC: []string{"reviewer1@kernel.org", "reviewer2@kernel.org"},
+ Date: time.Date(2000, 2, 9, 4, 5, 6, 7, time.UTC),
+ },
+ },
+ }
+ done.Build.ID = jobID
+ c.expectOK(c.client2.JobDone(done))
+
+ // The bisection result is unreliable - it shouldn't be reported.
+ c.expectNoEmail()
+
+ // Upload a crash with a C repro.
+ crash2 := testCrashWithRepro(build, 1)
+ c.client2.ReportCrash(crash2)
+
+ // Make sure it doesn't mention bisection and doesn't include the emails from it.
+ msg := c.pollEmailBug()
+ c.expectEQ(msg.To, []string{"test@syzkaller.com"})
+ c.expectEQ(msg.Subject, crash.Title)
+ c.expectTrue(strings.Contains(msg.Body, "syzbot has found a reproducer for the following issue"))
+ c.expectTrue(!strings.Contains(msg.Body, "bisection"))
+}
+
func TestBisectWrong(t *testing.T) {
// Test bisection results with BisectResultMerge/BisectResultNoop flags set.
// If any of these set, the result must not be reported separately,
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index 5bd890fb2..c9556c34c 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -379,11 +379,16 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key
if err != nil {
return nil, err
}
- job = job1
- if crash1.ReproC != 0 || crash.ReproC == 0 {
- // Don't override the crash in this case,
- // otherwise we will always think that we haven't reported the C repro.
- crash, crashKey = crash1, crashKey1
+ // If we didn't check whether the bisect is unreliable, even though it would not be
+ // reported anyway, we could still eventually Cc people from those commits later
+ // (e.g. when we did bisected with a syz repro and then notified about a C repro).
+ if !job1.isUnreliableBisect() {
+ job = job1
+ if crash1.ReproC != 0 || crash.ReproC == 0 {
+ // Don't override the crash in this case,
+ // otherwise we will always think that we haven't reported the C repro.
+ crash, crashKey = crash1, crashKey1
+ }
}
}
crashLog, _, err := getText(c, textCrashLog, crash.Log)