From 4e988e800e57d00326022e24ee940740e4b8e038 Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Thu, 18 Jan 2024 19:15:52 +0100 Subject: dashboard: retest missing backports Poll missing backport commits and, once a missing backport is found to be present in the fuzzed trees, don't display it on the web dashboard and send an email with the suggestion to close the bug. Closes #4425. --- dashboard/app/api.go | 37 +++++++++++++++++++ dashboard/app/app_test.go | 3 +- dashboard/app/config.go | 3 ++ dashboard/app/entities.go | 15 ++++++-- dashboard/app/jobs.go | 69 +++++++++++++++++++++++++++++++----- dashboard/app/mail_fix_candidate.txt | 23 +++++++++++- dashboard/app/tree_test.go | 60 +++++++++++++++++++++++++++++++ dashboard/dashapi/dashapi.go | 2 ++ 8 files changed, 200 insertions(+), 12 deletions(-) diff --git a/dashboard/app/api.go b/dashboard/app/api.go index ba6212da1..68a1ee192 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -263,9 +263,40 @@ func apiCommitPoll(c context.Context, ns string, r *http.Request, payload []byte for com := range commits { resp.Commits = append(resp.Commits, com) } + if getNsConfig(c, ns).RetestMissingBackports { + const takeBackportTitles = 5 + backportCommits, err := pollBackportCommits(c, ns, takeBackportTitles) + if err != nil { + return nil, err + } + resp.Commits = append(resp.Commits, backportCommits...) + } return resp, nil } +func pollBackportCommits(c context.Context, ns string, count int) ([]string, error) { + // Let's assume that there won't be too many pending backports. + bugs, jobs, _, err := relevantBackportJobs(c) + if err != nil { + return nil, fmt.Errorf("failed to query backport: %w", err) + } + var backportTitles []string + for i, bug := range bugs { + if bug.Namespace != ns { + continue + } + backportTitles = append(backportTitles, jobs[i].Commits[0].Title) + } + randomizer := rand.New(rand.NewSource(timeNow(c).UnixNano())) + randomizer.Shuffle(len(backportTitles), func(i, j int) { + backportTitles[i], backportTitles[j] = backportTitles[j], backportTitles[i] + }) + if len(backportTitles) > count { + backportTitles = backportTitles[:count] + } + return backportTitles, nil +} + func apiUploadCommits(c context.Context, ns string, r *http.Request, payload []byte) (interface{}, error) { req := new(dashapi.CommitPollResultReq) if err := json.Unmarshal(payload, req); err != nil { @@ -285,6 +316,12 @@ func apiUploadCommits(c context.Context, ns string, r *http.Request, payload []b return nil, err } } + if getNsConfig(c, ns).RetestMissingBackports { + err = updateBackportCommits(c, ns, req.Commits) + if err != nil { + return nil, fmt.Errorf("failed to update backport commits: %w", err) + } + } return nil, nil } diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go index a93a3e9ac..4e8e4efde 100644 --- a/dashboard/app/app_test.go +++ b/dashboard/app/app_test.go @@ -567,7 +567,8 @@ var testConfig = &GlobalConfig{ }, }, }, - FindBugOriginTrees: true, + FindBugOriginTrees: true, + RetestMissingBackports: true, }, }, } diff --git a/dashboard/app/config.go b/dashboard/app/config.go index 0fd47d2d0..f55a5f0ce 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -94,6 +94,9 @@ type Config struct { FixBisectionAutoClose bool // If set, dashboard will periodically request repros and revoke no longer working ones. RetestRepros bool + // If set, dashboard will periodically verify the presence of the missing backports in the + // tested kernel trees. + RetestMissingBackports bool // If set, dashboard will create patch testing jobs to determine bug origin trees. FindBugOriginTrees bool // Managers contains some special additional info about syz-manager instances. diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go index 74221af57..cdb8761dd 100644 --- a/dashboard/app/entities.go +++ b/dashboard/app/entities.go @@ -292,6 +292,16 @@ type Commit struct { Date time.Time } +func (com Commit) toDashapi() *dashapi.Commit { + return &dashapi.Commit{ + Hash: com.Hash, + Title: com.Title, + Author: com.Author, + AuthorName: com.AuthorName, + Date: com.Date, + } +} + type BugDiscussionInfo struct { Source string Summary DiscussionSummary @@ -588,8 +598,9 @@ type Job struct { Error int64 // reference to Error text entity, if set job failed Flags dashapi.JobDoneFlags - Reported bool // have we reported result back to user? - InvalidatedBy string // user who marked this bug as invalid, empty by default + Reported bool // have we reported result back to user? + InvalidatedBy string // user who marked this bug as invalid, empty by default + BackportedCommit Commit } func (job *Job) IsBisection() bool { diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go index 0b2298364..b7e24bdf7 100644 --- a/dashboard/app/jobs.go +++ b/dashboard/app/jobs.go @@ -1351,13 +1351,7 @@ func bisectFromJob(c context.Context, job *Job) (*dashapi.BisectResult, []string CrossTree: job.IsCrossTree(), } for _, com := range job.Commits { - bisect.Commits = append(bisect.Commits, &dashapi.Commit{ - Hash: com.Hash, - Title: com.Title, - Author: com.Author, - AuthorName: com.AuthorName, - Date: com.Date, - }) + bisect.Commits = append(bisect.Commits, com.toDashapi()) } var newEmails []string if len(bisect.Commits) == 1 { @@ -1367,6 +1361,9 @@ func bisectFromJob(c context.Context, job *Job) (*dashapi.BisectResult, []string newEmails = []string{com.Author} newEmails = append(newEmails, strings.Split(com.CC, "|")...) } + if job.BackportedCommit.Title != "" { + bisect.Backported = job.BackportedCommit.toDashapi() + } return bisect, newEmails } @@ -1654,7 +1651,8 @@ func relevantBackportJobs(c context.Context) ( err = fmt.Errorf("job %s: expected to be cross-tree", jobKey) return } - if len(job.Commits) != 1 || job.InvalidatedBy != "" { + if len(job.Commits) != 1 || job.InvalidatedBy != "" || + job.BackportedCommit.Title != "" { continue } bugs = append(bugs, allBugs[i]) @@ -1664,6 +1662,61 @@ func relevantBackportJobs(c context.Context) ( return } +func updateBackportCommits(c context.Context, ns string, commits []dashapi.Commit) error { + if len(commits) == 0 { + return nil + } + perTitle := map[string]dashapi.Commit{} + for _, commit := range commits { + perTitle[commit.Title] = commit + } + bugs, jobs, jobKeys, err := relevantBackportJobs(c) + if err != nil { + return fmt.Errorf("failed to query backport jobs: %w", err) + } + for i, job := range jobs { + rawCommit, ok := perTitle[job.Commits[0].Title] + if !ok { + continue + } + if bugs[i].Namespace != ns { + continue + } + commit := Commit{ + Hash: rawCommit.Hash, + Title: rawCommit.Title, + Author: rawCommit.Author, + AuthorName: rawCommit.AuthorName, + Date: rawCommit.Date, + } + err := commitBackported(c, jobKeys[i], commit) + if err != nil { + return fmt.Errorf("failed to update backport job: %w", err) + } + } + return nil +} + +func commitBackported(c context.Context, jobKey *db.Key, commit Commit) error { + tx := func(c context.Context) error { + job := new(Job) + if err := db.Get(c, jobKey, job); err != nil { + return fmt.Errorf("failed to get job: %w", err) + } + if job.BackportedCommit.Title != "" { + // Nothing to update. + return nil + } + job.BackportedCommit = commit + job.Reported = false + if _, err := db.Put(c, jobKey, job); err != nil { + return fmt.Errorf("failed to put job: %w", err) + } + return nil + } + return db.RunInTransaction(c, tx, &db.TransactionOptions{Attempts: 5}) +} + type bugJobs struct { list []*bugJob } diff --git a/dashboard/app/mail_fix_candidate.txt b/dashboard/app/mail_fix_candidate.txt index af247af62..83f3ca0d5 100644 --- a/dashboard/app/mail_fix_candidate.txt +++ b/dashboard/app/mail_fix_candidate.txt @@ -1,4 +1,22 @@ -{{$bisect := .BisectFix}}syzbot suspects this issue could be fixed by backporting the following commit: +{{$bisect := .BisectFix -}} + +{{- if $bisect.Backported -}} +The commit that was suspected to fix the issue was backported to the fuzzed +kernel trees. + +commit {{$bisect.Backported.Hash}} +Author: {{$bisect.Backported.AuthorName}} <{{$bisect.Backported.Author}}> +Date: {{formatKernelTime $bisect.Backported.Date}} + + {{$bisect.Commit.Title}} + +If you believe this is correct, please reply with +#syz fix: {{$bisect.Backported.Title}} + +The commit was initially detected here: +{{- else -}} +syzbot suspects this issue could be fixed by backporting the following commit: +{{- end}} commit {{$bisect.Commit.Hash}} git tree: {{.KernelRepoAlias}} @@ -17,6 +35,9 @@ bisection log: {{$bisect.LogLink}} {{end}}{{if .ReproCLink}}C reproducer: {{.ReproCLink}} {{end}} +{{- if not $bisect.Backported}} + Please keep in mind that other backports might be required as well. For information about bisection process see: https://goo.gl/tpsmEJ#bisection +{{end -}} diff --git a/dashboard/app/tree_test.go b/dashboard/app/tree_test.go index 5b8fc6134..f12799b42 100644 --- a/dashboard/app/tree_test.go +++ b/dashboard/app/tree_test.go @@ -198,6 +198,9 @@ func TestTreeOriginLts(t *testing.T) { ctx.ctx.expectNoEmail() } +// This function is very very big, but the required scenario is unfortunately +// also very big, so: +// nolint: funlen func TestTreeOriginLtsBisection(t *testing.T) { c := NewCtx(t) defer c.Close() @@ -361,6 +364,63 @@ For information about bisection process see: %URL%#bisection reply, err = ctx.ctx.AuthGET(AccessPublic, "/access-public-email/backports") c.expectOK(err) assert.NotContains(t, string(reply), treeTestCrashTitle) + + // The bug must appear in commit poll. + commitPollResp, err := ctx.client.CommitPoll() + c.expectOK(err) + assert.Contains(t, commitPollResp.Commits, "kernel: fix a bug") + + // Pretend that we have found a commit. + c.expectOK(ctx.client.UploadCommits([]dashapi.Commit{ + { + Hash: "newhash", + Title: "kernel: fix a bug", + AuthorName: "Someone", + Author: "someone@somewhere.com", + Date: time.Date(2000, 3, 4, 5, 6, 7, 8, time.UTC), + }, + })) + + // An email must be sent. + msg = ctx.emailWithoutURLs() + fmt.Printf("%s", msg) + c.expectEQ(msg.Body, `The commit that was suspected to fix the issue was backported to the fuzzed +kernel trees. + +commit newhash +Author: Someone +Date: Sat Mar 4 05:06:07 2000 +0000 + + kernel: fix a bug + +If you believe this is correct, please reply with +#syz fix: kernel: fix a bug + +The commit was initially detected here: + +commit deadf00d +git tree: upstream +Author: Someone +Date: Wed Feb 9 04:05:06 2000 +0000 + + kernel: fix a bug + +bisection log: %URL% +final oops: %URL% +console output: %URL% +kernel config: %URL% +dashboard link: %URL% +syz repro: %URL% +C reproducer: %URL% +`) + // Only one email. + ctx.ctx.expectNoEmail() + + // The commit should disappear from the missing backports list. + reply, err = ctx.ctx.AuthGET(AccessAdmin, "/tree-tests/backports") + c.expectOK(err) + assert.NotContains(t, string(reply), treeTestCrashTitle) + assert.NotContains(t, string(reply), "deadf00d") } func TestNonfinalFixCandidateBisect(t *testing.T) { diff --git a/dashboard/dashapi/dashapi.go b/dashboard/dashapi/dashapi.go index 37aebc631..96bbe7e5c 100644 --- a/dashboard/dashapi/dashapi.go +++ b/dashboard/dashapi/dashapi.go @@ -500,6 +500,8 @@ type BisectResult struct { CrashReportLink string Fix bool CrossTree bool + // In case a missing backport was backported. + Backported *Commit } type BugListReport struct { -- cgit mrf-deployment