From 7f0128ac333b2d71c7edb11465b933dedc9accf7 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 10 Mar 2019 12:08:24 -0400 Subject: Fix a potential segfault during infra container create I was seeing some segfaults where image config was being passed as nil, causing a nil dereference segfault. Fix the apparent cause and add some safety fencing to try and ensure it doesn't happen again. Signed-off-by: Matthew Heon --- libpod/runtime_pod_infra_linux.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) (limited to 'libpod/runtime_pod_infra_linux.go') diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go index 81579db4b..71328905b 100644 --- a/libpod/runtime_pod_infra_linux.go +++ b/libpod/runtime_pod_infra_linux.go @@ -31,17 +31,24 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, imgID isRootless := rootless.IsRootless() entryCmd := []string{r.config.InfraCommand} - // default to entrypoint in image if there is one - if len(config.Entrypoint) > 0 { - entryCmd = config.Entrypoint + if config == nil { + } - if len(config.Env) > 0 { - for _, nameValPair := range config.Env { - nameValSlice := strings.Split(nameValPair, "=") - if len(nameValSlice) < 2 { - return nil, errors.Errorf("Invalid environment variable structure in pause image") + // I've seen circumstances where config is being passed as nil. + // Let's err on the side of safety and make sure it's safe to use. + if config != nil { + // default to entrypoint in image if there is one + if len(config.Entrypoint) > 0 { + entryCmd = config.Entrypoint + } + if len(config.Env) > 0 { + for _, nameValPair := range config.Env { + nameValSlice := strings.Split(nameValPair, "=") + if len(nameValSlice) < 2 { + return nil, errors.Errorf("Invalid environment variable structure in pause image") + } + g.AddProcessEnv(nameValSlice[0], nameValSlice[1]) } - g.AddProcessEnv(nameValSlice[0], nameValSlice[1]) } } @@ -97,5 +104,5 @@ func (r *Runtime) createInfraContainer(ctx context.Context, p *Pod) (*Container, imageName := newImage.Names()[0] imageID := data.ID - return r.makeInfraContainer(ctx, p, imageName, imageID, newImage.Config) + return r.makeInfraContainer(ctx, p, imageName, imageID, data.Config) } -- cgit v1.2.3-54-g00ecf From eb3dd9415905b0b66dd8ce30cb6be21939fb4454 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 10 Mar 2019 12:20:04 -0400 Subject: Remove an unused if statement I added Signed-off-by: Matthew Heon --- libpod/runtime_pod_infra_linux.go | 3 --- 1 file changed, 3 deletions(-) (limited to 'libpod/runtime_pod_infra_linux.go') diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go index 71328905b..6b07bcb31 100644 --- a/libpod/runtime_pod_infra_linux.go +++ b/libpod/runtime_pod_infra_linux.go @@ -31,9 +31,6 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, imgID isRootless := rootless.IsRootless() entryCmd := []string{r.config.InfraCommand} - if config == nil { - - } // I've seen circumstances where config is being passed as nil. // Let's err on the side of safety and make sure it's safe to use. if config != nil { -- cgit v1.2.3-54-g00ecf From 54fd1a7bb962fbce828dd2fb2295518e3c706d8f Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 10 Mar 2019 14:25:13 -0400 Subject: Fix generation of infra container command When sourcing from an image, we need to grab its entrypoint first and then add command on to mimic the behavior of Docker. The default Kube pause image just sets ENTRYPOINT, and not CMD, so nothing changes there, but this ought to fix other images (for example, nginx would try to run the pause command instead of an nginx process without this patch) Signed-off-by: Matthew Heon --- libpod/runtime_pod_infra_linux.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'libpod/runtime_pod_infra_linux.go') diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go index 6b07bcb31..0a5f78cf8 100644 --- a/libpod/runtime_pod_infra_linux.go +++ b/libpod/runtime_pod_infra_linux.go @@ -12,6 +12,7 @@ import ( spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) const ( @@ -34,9 +35,24 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, imgID // I've seen circumstances where config is being passed as nil. // Let's err on the side of safety and make sure it's safe to use. if config != nil { + setEntrypoint := false // default to entrypoint in image if there is one if len(config.Entrypoint) > 0 { entryCmd = config.Entrypoint + setEntrypoint = true + } + if len(config.Cmd) > 0 { + // We can't use the default pause command, since we're + // sourcing from the image. If we didn't already set an + // entrypoint, set one now. + if !setEntrypoint { + // Use the Docker default "/bin/sh -c" + // entrypoint, as we're overriding command. + // If an image doesn't want this, it can + // override entrypoint too. + entryCmd = []string{"/bin/sh", "-c"} + } + entryCmd = append(entryCmd, config.Cmd...) } if len(config.Env) > 0 { for _, nameValPair := range config.Env { @@ -52,6 +68,8 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, imgID g.SetRootReadonly(true) g.SetProcessArgs(entryCmd) + logrus.Debugf("Using %q as infra container entrypoint", entryCmd) + if isRootless { g.RemoveMount("/dev/pts") devPts := spec.Mount{ -- cgit v1.2.3-54-g00ecf