From 808b223536e6553ddee2944d7ec6bc6d86e0da88 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Thu, 28 Mar 2019 01:07:58 -0700 Subject: [PATCH] Fix race and panic. Signed-off-by: Lantao Liu --- oci/spec_opts.go | 12 ++++++---- oci/spec_opts_test.go | 54 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/oci/spec_opts.go b/oci/spec_opts.go index 84a589933..ce756108a 100644 --- a/oci/spec_opts.go +++ b/oci/spec_opts.go @@ -741,7 +741,9 @@ func WithCapabilities(caps []string) SpecOpts { } // WithAllCapabilities sets all linux capabilities for the process -var WithAllCapabilities = WithCapabilities(GetAllCapabilities()) +var WithAllCapabilities = func(ctx context.Context, client Client, c *containers.Container, s *Spec) error { + return WithCapabilities(GetAllCapabilities())(ctx, client, c, s) +} // GetAllCapabilities returns all caps up to CAP_LAST_CAP // or CAP_BLOCK_SUSPEND on RHEL6 @@ -771,12 +773,14 @@ func capsContain(caps []string, s string) bool { } func removeCap(caps *[]string, s string) { - for i, c := range *caps { + var newcaps []string + for _, c := range *caps { if c == s { - *caps = append((*caps)[:i], (*caps)[i+1:]...) - return + continue } + newcaps = append(newcaps, c) } + *caps = newcaps } // WithAddedCapabilities adds the provided capabilities diff --git a/oci/spec_opts_test.go b/oci/spec_opts_test.go index 7cc65af94..b5193a832 100644 --- a/oci/spec_opts_test.go +++ b/oci/spec_opts_test.go @@ -166,25 +166,25 @@ func TestWithEnv(t *testing.T) { Env: []string{"DEFAULT=test"}, } - WithEnv([]string{"env=1"})(nil, nil, nil, &s) + WithEnv([]string{"env=1"})(context.Background(), nil, nil, &s) if len(s.Process.Env) != 2 { t.Fatal("didn't append") } - WithEnv([]string{"env2=1"})(nil, nil, nil, &s) + WithEnv([]string{"env2=1"})(context.Background(), nil, nil, &s) if len(s.Process.Env) != 3 { t.Fatal("didn't append") } - WithEnv([]string{"env2=2"})(nil, nil, nil, &s) + WithEnv([]string{"env2=2"})(context.Background(), nil, nil, &s) if s.Process.Env[2] != "env2=2" { t.Fatal("couldn't update") } - WithEnv([]string{"env2"})(nil, nil, nil, &s) + WithEnv([]string{"env2"})(context.Background(), nil, nil, &s) if len(s.Process.Env) != 2 { t.Fatal("couldn't unset") @@ -428,7 +428,7 @@ func TestAddCaps(t *testing.T) { var s specs.Spec - if err := WithAddedCapabilities([]string{"CAP_CHOWN"})(nil, nil, nil, &s); err != nil { + if err := WithAddedCapabilities([]string{"CAP_CHOWN"})(context.Background(), nil, nil, &s); err != nil { t.Fatal(err) } for i, cl := range [][]string{ @@ -448,10 +448,10 @@ func TestDropCaps(t *testing.T) { var s specs.Spec - if err := WithAllCapabilities(nil, nil, nil, &s); err != nil { + if err := WithAllCapabilities(context.Background(), nil, nil, &s); err != nil { t.Fatal(err) } - if err := WithDroppedCapabilities([]string{"CAP_CHOWN"})(nil, nil, nil, &s); err != nil { + if err := WithDroppedCapabilities([]string{"CAP_CHOWN"})(context.Background(), nil, nil, &s); err != nil { t.Fatal(err) } @@ -465,4 +465,44 @@ func TestDropCaps(t *testing.T) { t.Errorf("cap list %d contains dropped cap", i) } } + + // Add all capabilities back and drop a different cap. + if err := WithAllCapabilities(context.Background(), nil, nil, &s); err != nil { + t.Fatal(err) + } + if err := WithDroppedCapabilities([]string{"CAP_FOWNER"})(context.Background(), nil, nil, &s); err != nil { + t.Fatal(err) + } + + for i, cl := range [][]string{ + s.Process.Capabilities.Bounding, + s.Process.Capabilities.Effective, + s.Process.Capabilities.Permitted, + s.Process.Capabilities.Inheritable, + } { + if capsContain(cl, "CAP_FOWNER") { + t.Errorf("cap list %d contains dropped cap", i) + } + if !capsContain(cl, "CAP_CHOWN") { + t.Errorf("cap list %d doesn't contain non-dropped cap", i) + } + } + + // Drop all duplicated caps. + if err := WithCapabilities([]string{"CAP_CHOWN", "CAP_CHOWN"})(context.Background(), nil, nil, &s); err != nil { + t.Fatal(err) + } + if err := WithDroppedCapabilities([]string{"CAP_CHOWN"})(context.Background(), nil, nil, &s); err != nil { + t.Fatal(err) + } + for i, cl := range [][]string{ + s.Process.Capabilities.Bounding, + s.Process.Capabilities.Effective, + s.Process.Capabilities.Permitted, + s.Process.Capabilities.Inheritable, + } { + if len(cl) != 0 { + t.Errorf("cap list %d is not empty", i) + } + } }