From 0f90cc1ecb2db92e5388e07b8662b6c4a3a64f6c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin 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 --- 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"))