aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDmitry Vyukov <dvyukov@google.com>2017-02-02 20:21:38 +0100
committerDmitry Vyukov <dvyukov@google.com>2017-02-02 20:23:40 +0100
commit355865377102649df438e5d9dc7aa6d2a09bf7fc (patch)
treeb04295b486da771a0d0d2618fa27a5eaf69dfb0b
parentb9371a610178a570b770473c9654d1569cc00cee (diff)
vm: properly detect when a program exits
syz-fuzzer never exits (normally) so this does not affect syz-manager. But during reproduction we can run a short running program (no repeat mode) and currently VMs treat premature exit as an error. Properly detect when a program exits and let callers decide what to do with it.
-rw-r--r--vm/adb/adb.go9
-rw-r--r--vm/gce/gce.go21
-rw-r--r--vm/qemu/qemu.go7
-rw-r--r--vm/vm.go9
4 files changed, 36 insertions, 10 deletions
diff --git a/vm/adb/adb.go b/vm/adb/adb.go
index e634e671a..cb2f5ae33 100644
--- a/vm/adb/adb.go
+++ b/vm/adb/adb.go
@@ -379,9 +379,16 @@ func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command strin
}
signal(fmt.Errorf("instance closed"))
case err := <-merger.Err:
+ adb.Process.Kill()
+ merger.Wait()
+ if cmdErr := adb.Wait(); cmdErr == nil {
+ // If the command exited successfully, we got EOF error from merger.
+ // But in this case no error has happened and the EOF is expected.
+ err = nil
+ }
signal(err)
+ return
}
- tty.Close()
adb.Process.Kill()
merger.Wait()
adb.Wait()
diff --git a/vm/gce/gce.go b/vm/gce/gce.go
index edce6c38e..364a44f29 100644
--- a/vm/gce/gce.go
+++ b/vm/gce/gce.go
@@ -214,13 +214,24 @@ func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command strin
case <-inst.closed:
signal(fmt.Errorf("instance closed"))
case err := <-merger.Err:
- // Check if the instance was terminated due to preemption or host maintenance.
- time.Sleep(5 * time.Second) // just to avoid any GCE races
- if !GCE.IsInstanceRunning(inst.name) {
- Logf(1, "%v: ssh exited but instance is not running", inst.name)
- err = vm.TimeoutErr
+ con.Process.Kill()
+ ssh.Process.Kill()
+ merger.Wait()
+ con.Wait()
+ if cmdErr := ssh.Wait(); cmdErr == nil {
+ // If the command exited successfully, we got EOF error from merger.
+ // But in this case no error has happened and the EOF is expected.
+ err = nil
+ } else {
+ // Check if the instance was terminated due to preemption or host maintenance.
+ time.Sleep(5 * time.Second) // just to avoid any GCE races
+ if !GCE.IsInstanceRunning(inst.name) {
+ Logf(1, "%v: ssh exited but instance is not running", inst.name)
+ err = vm.TimeoutErr
+ }
}
signal(err)
+ return
}
con.Process.Kill()
ssh.Process.Kill()
diff --git a/vm/qemu/qemu.go b/vm/qemu/qemu.go
index c26620cf3..d9c37c822 100644
--- a/vm/qemu/qemu.go
+++ b/vm/qemu/qemu.go
@@ -354,7 +354,14 @@ func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command strin
case <-stop:
signal(vm.TimeoutErr)
case err := <-inst.merger.Err:
+ cmd.Process.Kill()
+ if cmdErr := cmd.Wait(); cmdErr == nil {
+ // If the command exited successfully, we got EOF error from merger.
+ // But in this case no error has happened and the EOF is expected.
+ err = nil
+ }
signal(err)
+ return
}
cmd.Process.Kill()
cmd.Wait()
diff --git a/vm/vm.go b/vm/vm.go
index ce4138344..79fdc4f76 100644
--- a/vm/vm.go
+++ b/vm/vm.go
@@ -119,7 +119,7 @@ func MonitorExecution(outc <-chan []byte, errc <-chan error, local, needOutput b
return "preempted", nil, nil, false, true
}
if !report.ContainsCrash(output[matchPos:], ignores) {
- return defaultError, nil, output, true, false
+ return defaultError, nil, output, defaultError != "", false
}
desc, text, start, end := report.Parse(output[matchPos:], ignores)
start = start + matchPos - beforeContext
@@ -146,8 +146,9 @@ func MonitorExecution(outc <-chan []byte, errc <-chan error, local, needOutput b
case err := <-errc:
switch err {
case nil:
- waitForOutput()
- return "", nil, output, false, false
+ // The program has exited without errors,
+ // but wait for kernel output in case there is some delayed oops.
+ return extractError("")
case TimeoutErr:
return err.Error(), nil, nil, false, true
default:
@@ -164,7 +165,7 @@ func MonitorExecution(outc <-chan []byte, errc <-chan error, local, needOutput b
lastExecuteTime = time.Now()
}
if report.ContainsCrash(output[matchPos:], ignores) {
- return extractError("")
+ return extractError("unknown error")
}
if len(output) > 2*beforeContext {
copy(output, output[len(output)-beforeContext:])