diff --git a/dashboard/app/api.go b/dashboard/app/api.go index 41280227..5ccb3c18 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -233,7 +233,8 @@ func apiUploadBuild(c context.Context, ns string, r *http.Request, payload []byt if err := json.Unmarshal(payload, req); err != nil { return nil, fmt.Errorf("failed to unmarshal request: %v", err) } - isNewBuild, err := uploadBuild(c, ns, req, BuildNormal) + now := timeNow(c) + isNewBuild, err := uploadBuild(c, now, ns, req, BuildNormal) if err != nil { return nil, err } @@ -253,7 +254,7 @@ func apiUploadBuild(c context.Context, ns string, r *http.Request, payload []byt return nil, nil } -func uploadBuild(c context.Context, ns string, req *dashapi.Build, typ BuildType) (bool, error) { +func uploadBuild(c context.Context, now time.Time, ns string, req *dashapi.Build, typ BuildType) (bool, error) { if _, err := loadBuild(c, ns, req.ID); err == nil { return false, nil } @@ -276,8 +277,8 @@ func uploadBuild(c context.Context, ns string, req *dashapi.Build, typ BuildType if err := checkStrLen(req.KernelRepo, "Build.KernelRepo", MaxStringLen); err != nil { return false, err } - if err := checkStrLen(req.KernelBranch, "Build.KernelBranch", MaxStringLen); err != nil { - return false, err + if len(req.KernelBranch) > MaxStringLen { + return false, fmt.Errorf("Build.KernelBranch is too long (%v)", len(req.KernelBranch)) } if err := checkStrLen(req.SyzkallerCommit, "Build.SyzkallerCommit", MaxStringLen); err != nil { return false, err @@ -297,7 +298,7 @@ func uploadBuild(c context.Context, ns string, req *dashapi.Build, typ BuildType Manager: req.Manager, ID: req.ID, Type: typ, - Time: timeNow(c), + Time: now, OS: req.OS, Arch: req.Arch, VMArch: req.VMArch, @@ -450,7 +451,8 @@ func apiReportBuildError(c context.Context, ns string, r *http.Request, payload if err := json.Unmarshal(payload, req); err != nil { return nil, fmt.Errorf("failed to unmarshal request: %v", err) } - if _, err := uploadBuild(c, ns, &req.Build, BuildFailed); err != nil { + now := timeNow(c) + if _, err := uploadBuild(c, now, ns, &req.Build, BuildFailed); err != nil { return nil, err } req.Crash.BuildID = req.Build.ID diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go index ebfbbf70..9e558e4e 100644 --- a/dashboard/app/entities.go +++ b/dashboard/app/entities.go @@ -355,7 +355,10 @@ func kernelRepoInfo(build *Build) KernelRepo { } func kernelRepoInfoRaw(repo, branch string) KernelRepo { - repoID := repo + "/" + branch + repoID := repo + if branch != "" { + repoID += "/" + branch + } info := config.KernelRepos[repoID] if info.Alias == "" { info.Alias = repoID diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go index e1a5f4d6..d085b1f5 100644 --- a/dashboard/app/jobs.go +++ b/dashboard/app/jobs.go @@ -87,10 +87,8 @@ func addTestJob(c context.Context, bug *Bug, bugKey *datastore.Key, bugReporting switch { case !git.CheckRepoAddress(repo): return fmt.Sprintf("%q does not look like a valid git repo address.", repo), nil - case !git.CheckBranch(branch): - return fmt.Sprintf("%q does not look like a valid git branch name.", branch), nil - case len(patch) == 0: - return "I don't see any patch attached to the request.", nil + case !git.CheckBranch(branch) && !git.CheckCommitHash(branch): + return fmt.Sprintf("%q does not look like a valid git branch or commit.", branch), nil case crash.ReproC == 0 && crash.ReproSyz == 0: return "This crash does not have a reproducer. I cannot test it.", nil case bug.Status == BugStatusFixed: @@ -164,7 +162,7 @@ func addTestJob(c context.Context, bug *Bug, bugKey *datastore.Key, bugReporting return nil } err = datastore.RunInTransaction(c, tx, &datastore.TransactionOptions{XG: true, Attempts: 30}) - if deletePatch || err != nil { + if patchID != 0 && deletePatch || err != nil { datastore.Delete(c, datastore.NewKey(c, textPatch, "", patchID, nil)) } if err != nil { @@ -268,7 +266,7 @@ func doneJob(c context.Context, req *dashapi.JobDoneReq) error { return fmt.Errorf("job %v: already finished", jobID) } ns := job.Namespace - if isNewBuild, err := uploadBuild(c, ns, &req.Build, BuildJob); err != nil { + if isNewBuild, err := uploadBuild(c, now, ns, &req.Build, BuildJob); err != nil { return err } else if !isNewBuild { log.Errorf(c, "job %v: duplicate build %v", jobID, req.Build.ID) diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go index 218ab63f..6c82798c 100644 --- a/dashboard/app/jobs_test.go +++ b/dashboard/app/jobs_test.go @@ -78,11 +78,6 @@ func TestJob(t *testing.T) { c.expectEQ(len(c.emailSink), 1) c.expectEQ(strings.Contains((<-c.emailSink).Body, "does not look like a valid git repo"), true) - c.incomingEmail(sender, "#syz test: git://git.git/git.git master", - EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) - c.expectEQ(len(c.emailSink), 1) - c.expectEQ(strings.Contains((<-c.emailSink).Body, "I don't see any patch attached to the request"), true) - c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch, EmailOptFrom("\"foo\" ")) c.expectOK(c.GET("/email_poll")) @@ -200,7 +195,6 @@ compiler: compiler1 Patch: %[1]v Kernel config: %[2]v - `, patchLink, kernelConfigLink) if msg.Body != body { t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) @@ -246,7 +240,6 @@ compiler: compiler1 Patch: %[3]v Kernel config: %[4]v - `, truncatedError, errorLink, patchLink, kernelConfigLink) if msg.Body != body { t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) @@ -287,7 +280,6 @@ compiler: compiler1 Patch: %[2]v Kernel config: %[3]v - --- There is no WARRANTY for the result, to the extent permitted by applicable law. Except when otherwise stated in writing syzbot provides the result "AS IS" @@ -307,3 +299,79 @@ correction. c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) c.expectEQ(pollResp.ID, "") } + +func TestJobWithoutPatch(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build := testBuild(1) + c.expectOK(c.API(client2, key2, "upload_build", build, nil)) + + crash := testCrash(build, 1) + crash.ReproOpts = []byte("repro opts") + crash.ReproSyz = []byte("repro syz") + crash.ReproC = []byte("repro C") + crash.Maintainers = []string{"maintainer@kernel.org"} + c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 1) + sender := (<-c.emailSink).Sender + _, extBugID, err := email.RemoveAddrContext(sender) + if err != nil { + t.Fatal(err) + } + + // Test on particular commit and without a patch. + c.incomingEmail(sender, "#syz test: git://mygit.com/git.git 5e6a2eea\n", EmailOptMessageID(1)) + pollResp := new(dashapi.JobPollResp) + c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + testBuild := testBuild(2) + testBuild.KernelRepo = "git://mygit.com/git.git" + testBuild.KernelBranch = "" + testBuild.KernelCommit = "5e6a2eea5e6a2eea5e6a2eea5e6a2eea5e6a2eea" + jobDoneReq := &dashapi.JobDoneReq{ + ID: pollResp.ID, + Build: *testBuild, + } + c.expectOK(c.API(client2, key2, "job_done", jobDoneReq, nil)) + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 1) + { + _, dbBuild := c.loadJob(pollResp.ID) + kernelConfigLink := externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig) + msg := <-c.emailSink + c.expectEQ(len(msg.Attachments), 0) + body := fmt.Sprintf(`Hello, + +syzbot has tested the proposed patch and the reproducer did not trigger crash: + +Reported-and-tested-by: syzbot+%v@testapp.appspotmail.com + +Note: the tag will also help syzbot to understand when the bug is fixed. + +Tested on git://mygit.com/git.git commit +5e6a2eea5e6a2eea5e6a2eea5e6a2eea5e6a2eea (Sat Feb 3 04:05:06 0001 +0000) +kernel_commit_title2 + +compiler: compiler2 +Kernel config: %[2]v + +--- +There is no WARRANTY for the result, to the extent permitted by applicable law. +Except when otherwise stated in writing syzbot provides the result "AS IS" +without warranty of any kind, either expressed or implied, but not limited to, +the implied warranties of merchantability and fittness for a particular purpose. +The entire risk as to the quality of the result is with you. Should the result +prove defective, you assume the cost of all necessary servicing, repair or +correction. +`, extBugID, kernelConfigLink) + if msg.Body != body { + t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) + } + c.checkURLContents(kernelConfigLink, testBuild.KernelConfig) + } + + c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + c.expectEQ(pollResp.ID, "") +} diff --git a/dashboard/app/mail_test_result.txt b/dashboard/app/mail_test_result.txt index a49475cf..f39ce470 100644 --- a/dashboard/app/mail_test_result.txt +++ b/dashboard/app/mail_test_result.txt @@ -24,10 +24,10 @@ Tested on {{.KernelRepo}} commit {{.KernelCommitTitle}}{{end}} compiler: {{.CompilerID}} -{{if .PatchLink}}Patch: {{.PatchLink}}{{end}} -{{if .KernelConfigLink}}Kernel config: {{.KernelConfigLink}}{{end}} -{{if .LogLink}}Raw console output: {{.LogLink}}{{end}} -{{if and (not .CrashTitle) (not .Error)}} +{{if .PatchLink}}Patch: {{.PatchLink}} +{{end}}{{if .KernelConfigLink}}Kernel config: {{.KernelConfigLink}} +{{end}}{{if .LogLink}}Raw console output: {{.LogLink}} +{{end}}{{if and (not .CrashTitle) (not .Error)}} --- There is no WARRANTY for the result, to the extent permitted by applicable law. Except when otherwise stated in writing syzbot provides the result "AS IS" diff --git a/pkg/git/git.go b/pkg/git/git.go index 37230963..77a2630f 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -57,9 +57,9 @@ func Poll(dir, repo, branch string) (*Commit, error) { return HeadCommit(dir) } -// Checkout checkouts the specified repository/branch in dir. +// CheckoutBranch checkouts the specified repository/branch in dir. // It does not fetch history and efficiently supports checkouts of different repos in the same dir. -func Checkout(dir, repo, branch string) (*Commit, error) { +func CheckoutBranch(dir, repo, branch string) (*Commit, error) { if _, err := runSandboxed(dir, "git", "reset", "--hard"); err != nil { if err := initRepo(dir); err != nil { return nil, err @@ -75,21 +75,21 @@ func Checkout(dir, repo, branch string) (*Commit, error) { return HeadCommit(dir) } -// CheckoutCommit checkouts the specified repository/branch/commit in dir. -func CheckoutCommit(dir, repo, branch, commit string) error { +// CheckoutCommit checkouts the specified repository on the specified commit in dir. +func CheckoutCommit(dir, repo, commit string) (*Commit, error) { if _, err := runSandboxed(dir, "git", "reset", "--hard"); err != nil { if err := initRepo(dir); err != nil { - return err + return nil, err } } - _, err := runSandboxed(dir, "git", "fetch", repo, branch) + _, err := runSandboxed(dir, "git", "fetch", repo) if err != nil { - return err + return nil, err } if _, err := runSandboxed(dir, "git", "checkout", commit); err != nil { - return err + return nil, err } - return nil + return HeadCommit(dir) } func clone(dir, repo, branch string) error { @@ -322,11 +322,21 @@ func CheckRepoAddress(repo string) bool { return gitRepoRe.MatchString(repo) } -var gitRepoRe = regexp.MustCompile(`^(git|ssh|http|https|ftp|ftps)://[a-zA-Z0-9-_]+(\.[a-zA-Z0-9-_]+)+(:[0-9]+)?/[a-zA-Z0-9-_./]+\.git(/)?$`) - // CheckBranch does a best-effort approximate check of a git branch name. func CheckBranch(branch string) bool { return gitBranchRe.MatchString(branch) } -var gitBranchRe = regexp.MustCompile("^[a-zA-Z0-9-_/.]{2,200}$") +func CheckCommitHash(hash string) bool { + if !gitHashRe.MatchString(hash) { + return false + } + ln := len(hash) + return ln == 8 || ln == 10 || ln == 12 || ln == 16 || ln == 20 || ln == 40 +} + +var ( + gitRepoRe = regexp.MustCompile(`^(git|ssh|http|https|ftp|ftps)://[a-zA-Z0-9-_]+(\.[a-zA-Z0-9-_]+)+(:[0-9]+)?/[a-zA-Z0-9-_./]+\.git(/)?$`) + gitBranchRe = regexp.MustCompile("^[a-zA-Z0-9-_/.]{2,200}$") + gitHashRe = regexp.MustCompile("^[a-f0-9]+$") +) diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go index 8e23e84b..7ad42e3d 100644 --- a/pkg/git/git_test.go +++ b/pkg/git/git_test.go @@ -66,6 +66,7 @@ func TestCheckBranch(t *testing.T) { {"linux-4.9.y", true}, {"abi_spec", true}, {"@", false}, + {"", false}, } for _, test := range tests { res := CheckBranch(test.branch) @@ -75,6 +76,30 @@ func TestCheckBranch(t *testing.T) { } } +func TestCheckCommitHash(t *testing.T) { + var tests = []struct { + hash string + result bool + }{ + {"ff12bea91c22bba93d3ffc3034d813d686bc7eeb", true}, // 40 + {"eae05cb0aaeae05cb0aa", true}, // 20 + {"449dd6984d0eaabb", true}, // 16 + {"449dd6984d0e", true}, // 12 + {"eae05cb0aa", true}, // 10 + {"eae05cb0", true}, // 8 + {"", false}, + {"aa", false}, + {"eae05cb0aab", false}, + {"xxxxxxxx", false}, + } + for _, test := range tests { + res := CheckCommitHash(test.hash) + if res != test.result { + t.Errorf("%v: got %v, want %v", test.hash, res, test.result) + } + } +} + func TestExtractFixTags(t *testing.T) { commits, err := extractFixTags(strings.NewReader(extractFixTagsInput), extractFixTagsEmail) if err != nil { diff --git a/syz-ci/jobs.go b/syz-ci/jobs.go index 42f8e585..013836b5 100644 --- a/syz-ci/jobs.go +++ b/syz-ci/jobs.go @@ -199,7 +199,7 @@ func (jp *JobProcessor) buildImage(job *Job) error { } Logf(0, "job: fetching syzkaller on %v...", req.SyzkallerCommit) - err := git.CheckoutCommit(syzkallerDir, jp.syzkallerRepo, jp.syzkallerBranch, req.SyzkallerCommit) + _, err := git.CheckoutCommit(syzkallerDir, jp.syzkallerRepo, req.SyzkallerCommit) if err != nil { return fmt.Errorf("failed to checkout syzkaller repo: %v", err) } @@ -220,9 +220,20 @@ func (jp *JobProcessor) buildImage(job *Job) error { resp.Build.SyzkallerCommit = req.SyzkallerCommit Logf(0, "job: fetching kernel...") - kernelCommit, err := git.Checkout(kernelDir, req.KernelRepo, req.KernelBranch) - if err != nil { - return fmt.Errorf("failed to checkout kernel repo: %v", err) + var kernelCommit *git.Commit + if git.CheckCommitHash(req.KernelBranch) { + kernelCommit, err = git.CheckoutCommit(kernelDir, req.KernelRepo, req.KernelBranch) + if err != nil { + return fmt.Errorf("failed to checkout kernel repo %v on commit %v: %v", + req.KernelRepo, req.KernelBranch, err) + } + resp.Build.KernelBranch = "" + } else { + kernelCommit, err = git.CheckoutBranch(kernelDir, req.KernelRepo, req.KernelBranch) + if err != nil { + return fmt.Errorf("failed to checkout kernel repo %v/%v: %v", + req.KernelRepo, req.KernelBranch, err) + } } resp.Build.KernelCommit = kernelCommit.Hash resp.Build.KernelCommitTitle = kernelCommit.Title @@ -231,8 +242,10 @@ func (jp *JobProcessor) buildImage(job *Job) error { if err := kernel.Clean(kernelDir); err != nil { return fmt.Errorf("kernel clean failed: %v", err) } - if err := git.Patch(kernelDir, req.Patch); err != nil { - return err + if len(req.Patch) != 0 { + if err := git.Patch(kernelDir, req.Patch); err != nil { + return err + } } Logf(0, "job: building kernel...")