aboutsummaryrefslogtreecommitdiffstats
path: root/pkg/bisect
diff options
context:
space:
mode:
authorSpace Meyer <meyerpatrick@google.com>2022-08-12 14:32:53 +0000
committerAleksandr Nogikh <wp32pw@gmail.com>2022-09-01 10:42:40 +0200
commitd9393412ccc24ce82dcd6ab8916c966615150edd (patch)
tree62ac522ffe6a06f4932ae07b9c6b9bfe737bb1e2 /pkg/bisect
parentb01ec5715e2ae72463124fd42608bcee9b8132b8 (diff)
pkg/bisect: prevent crash in bisection after failed repo HEAD query
During a bisection the call stack looks like this: Starting at env.bisect() in pkg/bisect/bisect.go bisect() -> Bisect() -> pred() -> test() -> build() Previously the `results[testRes1.com.Hash]` in pred() could run into a nil pointer deref and crash, when: * build() didn't bother to return current in certain error conditions * test() didn't handle cases where build() couldn't query current When test() returns an error, the bisection it was called from is aborted. Hence test() is expected to not simply bubble up recoverable errors, but instead return a testResult with verdict=vcs.BisectSkip. I ran into this with a linux branch, where syzkaller could not find any release tags, running into one of the cases where build() did not return current, even though it was able to obtain it.
Diffstat (limited to 'pkg/bisect')
-rw-r--r--pkg/bisect/bisect.go10
1 files changed, 8 insertions, 2 deletions
diff --git a/pkg/bisect/bisect.go b/pkg/bisect/bisect.go
index 3320e7da3..bc442c16a 100644
--- a/pkg/bisect/bisect.go
+++ b/pkg/bisect/bisect.go
@@ -417,13 +417,13 @@ func (env *env) build() (*vcs.Commit, string, error) {
bisectEnv, err := env.bisecter.EnvForCommit(env.cfg.BinDir, current.Hash, env.kernelConfig)
if err != nil {
- return nil, "", err
+ return current, "", err
}
env.log("testing commit %v", current.Hash)
buildStart := time.Now()
mgr := env.cfg.Manager
if err := build.Clean(mgr.TargetOS, mgr.TargetVMArch, mgr.Type, mgr.KernelSrc); err != nil {
- return nil, "", fmt.Errorf("kernel clean failed: %v", err)
+ return current, "", fmt.Errorf("kernel clean failed: %v", err)
}
kern := &env.cfg.Kernel
_, imageDetails, err := env.inst.BuildKernel(bisectEnv.Compiler, env.cfg.Ccache, kern.Userspace,
@@ -438,6 +438,8 @@ func (env *env) build() (*vcs.Commit, string, error) {
return current, imageDetails.Signature, err
}
+// Note: When this function returns an error, the bisection it was called from is aborted.
+// Hence recoverable errors must be handled and the callers must treat testResult with care.
func (env *env) test() (*testResult, error) {
cfg := env.cfg
if cfg.Timeout != 0 && time.Since(env.startTime) > cfg.Timeout {
@@ -449,6 +451,10 @@ func (env *env) test() (*testResult, error) {
com: current,
kernelSign: kernelSign,
}
+ if current == nil {
+ // This is not recoverable, as the caller must know which commit to skip.
+ return res, fmt.Errorf("couldn't get repo HEAD: %v", err)
+ }
if err != nil {
if verr, ok := err.(*osutil.VerboseError); ok {
env.log("%v", verr.Title)