diff options
| author | Space Meyer <meyerpatrick@google.com> | 2022-08-12 14:32:53 +0000 |
|---|---|---|
| committer | Aleksandr Nogikh <wp32pw@gmail.com> | 2022-09-01 10:42:40 +0200 |
| commit | d9393412ccc24ce82dcd6ab8916c966615150edd (patch) | |
| tree | 62ac522ffe6a06f4932ae07b9c6b9bfe737bb1e2 /pkg/bisect | |
| parent | b01ec5715e2ae72463124fd42608bcee9b8132b8 (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.go | 10 |
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) |
