From 355865377102649df438e5d9dc7aa6d2a09bf7fc Mon Sep 17 00:00:00 2001
From: Dmitry Vyukov <dvyukov@google.com>
Date: Thu, 2 Feb 2017 20:21:38 +0100
Subject: 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.
---
 vm/adb/adb.go   |  9 ++++++++-
 vm/gce/gce.go   | 21 ++++++++++++++++-----
 vm/qemu/qemu.go |  7 +++++++
 vm/vm.go        |  9 +++++----
 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:])
-- 
cgit mrf-deployment