Blob Blame History Raw
From 0f90cc1ecb2db92e5388e07b8662b6c4a3a64f6c Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Tue, 15 Sep 2020 21:46:32 -0700
Subject: [PATCH] runc run: fix panic on error

In case (*initProcess).start did not set sentRun, and ierr is nil,
runc run panics:

```
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x38 pc=0x68a117]

goroutine 1 [running]:
github.com/urfave/cli.HandleAction.func1(0xc0002277d8)
	/home/kir/go/src/github.com/projectatomic/runc/Godeps/_workspace/src/github.com/urfave/cli/app.go:478 +0x22d
panic(0x730b60, 0xa06fc0)
	/usr/lib/golang/src/runtime/panic.go:969 +0x166
github.com/opencontainers/runc/libcontainer.(*genericError).Error(0x0, 0xc0002ca0e0, 0xe)
	/home/kir/go/src/github.com/projectatomic/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/generic_error.go:93 +0x37
github.com/opencontainers/runc/libcontainer.createSystemError(0x7fcd20, 0x0, 0x78c23e, 0xe, 0xc000098050, 0x0)
	/home/kir/go/src/github.com/projectatomic/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/generic_error.go:78 +0x14c
github.com/opencontainers/runc/libcontainer.newSystemErrorWithCause(...)
	/home/kir/go/src/github.com/projectatomic/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/generic_error.go:63
github.com/opencontainers/runc/libcontainer.(*initProcess).start(0xc000298000, 0x0, 0x0)
	/home/kir/go/src/github.com/projectatomic/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/process_linux.go:361 +0x94b
....
```

This is caused by the fact that `ierr` is a typed variable (rather than a
generic `error`), and when `newSystemErrorWithCause(ierr, ...)` is called
with a typed variable, the check `if err != nil` in `createSystemError`
does not work, since err has a type. This Golang peculiarity is described
in https://golang.org/doc/faq#nil_error.

After this patch (tested by temporarily modifying the source to set
`sentRun` to `false`) it no longer panics, instead we get:

```
container_linux.go:247: starting container process caused "container init failed"
```

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/process_linux.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go
index 7c92c93a..53df9fa5 100644
--- docker-0be3e217c42ecf554bf5117bec9c832bd3f3b6fd/runc-66aedde759f33c190954815fb765eedc1d782dd9/libcontainer/process_linux.go
+++ docker-0be3e217c42ecf554bf5117bec9c832bd3f3b6fd/runc-66aedde759f33c190954815fb765eedc1d782dd9/libcontainer/process_linux.go
@@ -364,7 +364,10 @@ loop:
 		return newSystemError(fmt.Errorf("container init exited prematurely"))
 	}
 	if !sentRun {
-		return newSystemErrorWithCause(ierr, "container init")
+		if ierr != nil {
+			return newSystemErrorWithCause(ierr, "container init")
+		}
+		return newSystemError(errors.New("container init failed"))
 	}
 	if p.config.Config.Namespaces.Contains(configs.NEWNS) && !sentResume {
 		return newSystemError(fmt.Errorf("could not synchronise after executing prestart hooks with container process"))