diff options
| author | Aleksandr Nogikh <nogikh@google.com> | 2022-05-30 13:17:24 +0000 |
|---|---|---|
| committer | Aleksandr Nogikh <wp32pw@gmail.com> | 2022-05-31 14:34:53 +0200 |
| commit | 3666edfeb55080ebe138d77417fa96fe2555d6bb (patch) | |
| tree | 6ab82b9954c03306a5262c5692fbb83da34d400a | |
| parent | af70c3a9d26d6637e932facd13b1e55dd96270b5 (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.go | 48 | ||||
| -rw-r--r-- | dashboard/app/reporting.go | 15 |
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) |
