diff --git a/CHANGELOG.md b/CHANGELOG.md index db1cd1924d6..80d6907f245 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 default umask of 022 was set. (#5479) ### Changed ### +- runc now rejects a new container, or a process passed to + `runc exec --process`, with duplicate `rlimits` entries of the same type, + as required by the runtime-spec. Previously, the last such entry silently + took effect. `runc exec` into an existing container is not affected. + (#5493, #5494) - runc now requires Go 1.26+ to build. (#5413) - Updated builds to libseccomp v2.6.1. (#5376) - Switched to opencontainers/cgroups v0.1.0, which no longer uses the diff --git a/exec.go b/exec.go index 8f744e60299..b5a14e4360d 100644 --- a/exec.go +++ b/exec.go @@ -217,7 +217,15 @@ func getProcess(cmd *cli.Command, c *libcontainer.Container) (*specs.Process, er if err := json.NewDecoder(f).Decode(&p); err != nil { return nil, err } - return &p, validateProcessSpec(&p) + if err := validateProcessSpec(&p); err != nil { + return nil, err + } + // The process is new input (not from an existing container's + // config.json), so reject duplicate rlimit types here. + if err := checkProcessRlimits(&p); err != nil { + return nil, err + } + return &p, nil } // Process from config.json and CLI flags. bundle, ok := utils.SearchLabels(c.Config().Labels, "bundle") diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go index 5306bfc200a..6cc33b5bb86 100644 --- a/libcontainer/configs/config.go +++ b/libcontainer/configs/config.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "os/exec" + "slices" "strconv" "strings" "time" @@ -633,3 +634,16 @@ func (c *Command) Run(s *specs.State) error { return fmt.Errorf("hook ran past specified timeout of %.1fs", c.Timeout.Seconds()) } } + +// CheckRlimits returns an error if limits contain more than one entry +// of the same type. +func CheckRlimits(limits []Rlimit) error { + // There are only a few rlimit types, so a linear search is cheaper + // than building a map. + for i, l := range limits { + if slices.ContainsFunc(limits[:i], func(r Rlimit) bool { return r.Type == l.Type }) { + return fmt.Errorf("duplicate rlimit type: %d", l.Type) + } + } + return nil +} diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go index 7674d3e6cc0..e200858644f 100644 --- a/libcontainer/configs/validate/validator.go +++ b/libcontainer/configs/validate/validator.go @@ -34,6 +34,7 @@ func Validate(config *configs.Config) error { scheduler, ioPriority, memoryPolicy, + rlimits, } for _, c := range checks { if err := c(config); err != nil { @@ -520,3 +521,9 @@ func memoryPolicy(config *configs.Config) error { } return nil } + +// rlimits checks that there is no more than one entry per rlimit type, +// as required by the runtime-spec. +func rlimits(config *configs.Config) error { + return configs.CheckRlimits(config.Rlimits) +} diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go index fd7a2633746..27c00c9d1e8 100644 --- a/libcontainer/configs/validate/validator_test.go +++ b/libcontainer/configs/validate/validator_test.go @@ -1168,3 +1168,39 @@ func TestDevValidName(t *testing.T) { }) } } + +func TestValidateRlimits(t *testing.T) { + for _, tc := range []struct { + name string + rlimits []configs.Rlimit + isErr bool + }{ + {name: "none"}, + { + name: "distinct", + rlimits: []configs.Rlimit{ + {Type: unix.RLIMIT_NOFILE, Soft: 32, Hard: 64}, + {Type: unix.RLIMIT_CORE}, + }, + }, + { + name: "duplicate", + rlimits: []configs.Rlimit{ + {Type: unix.RLIMIT_NOFILE, Soft: 32, Hard: 64}, + {Type: unix.RLIMIT_NOFILE, Soft: 48, Hard: 64}, + }, + isErr: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + config := &configs.Config{Rootfs: "/var", Rlimits: tc.rlimits} + err := Validate(config) + if tc.isErr && err == nil { + t.Fatal("expected error, got nil") + } + if !tc.isErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 5d97e9141d3..04412b0a494 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -376,6 +376,9 @@ func (c *Container) start(process *Process) (retErr error) { if c.initProcessStartTime != 0 { return errors.New("container already has init process") } + if err := configs.CheckRlimits(process.Rlimits); err != nil { + return err + } if err := c.createExecFifo(); err != nil { return err } diff --git a/tests/integration/exec.bats b/tests/integration/exec.bats index c9ad2f4860f..7f0586c45ff 100644 --- a/tests/integration/exec.bats +++ b/tests/integration/exec.bats @@ -366,3 +366,36 @@ EOF run -0 runc exec -u 2000 test sh -c "echo \$HOME" assert_line --index 0 "/home/tempuser" } + +# Duplicate rlimit types are rejected for new containers and for +# "runc exec --process", but not for an existing container whose +# config.json (created by an older runc) has them. +@test "runc exec [duplicate rlimits in existing container config]" { + run -0 runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + + # Simulate a config.json from an older runc. + update_config '.process.rlimits = [ + {"type": "RLIMIT_NOFILE", "soft": 32, "hard": 64}, + {"type": "RLIMIT_NOFILE", "soft": 48, "hard": 64} + ]' + + run -0 runc exec test_busybox true + + proc='{"terminal": false, "cwd": "/", "args": ["true"], + "rlimits": [ + {"type": "RLIMIT_NOFILE", "soft": 32, "hard": 64}, + {"type": "RLIMIT_NOFILE", "soft": 48, "hard": 64} + ]}' + run ! runc exec --process <(echo "$proc") test_busybox + assert_output --partial "duplicate rlimit type" +} + +@test "runc run [duplicate rlimits]" { + update_config '.process.rlimits = [ + {"type": "RLIMIT_NOFILE", "soft": 32, "hard": 64}, + {"type": "RLIMIT_NOFILE", "soft": 48, "hard": 64} + ]' + + run ! runc run test_busybox + assert_output --partial "duplicate rlimit type" +} diff --git a/utils_linux.go b/utils_linux.go index 1da1c872d88..af1abe9c566 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -7,6 +7,7 @@ import ( "net" "os" "path/filepath" + "slices" "strconv" "github.com/opencontainers/runtime-spec/specs-go" @@ -351,6 +352,16 @@ func (r *runner) checkTerminal(config *specs.Process) error { return nil } +// checkProcessRlimits returns an error if p has duplicate rlimit types. +func checkProcessRlimits(p *specs.Process) error { + for i, r := range p.Rlimits { + if slices.ContainsFunc(p.Rlimits[:i], func(x specs.POSIXRlimit) bool { return x.Type == r.Type }) { + return fmt.Errorf("duplicate rlimit type: %s", r.Type) + } + } + return nil +} + func validateProcessSpec(spec *specs.Process) error { if spec == nil { return errors.New("process property must not be empty") diff --git a/utils_linux_test.go b/utils_linux_test.go new file mode 100644 index 00000000000..ece70229d80 --- /dev/null +++ b/utils_linux_test.go @@ -0,0 +1,42 @@ +package main + +import ( + "testing" + + "github.com/opencontainers/runtime-spec/specs-go" +) + +func TestCheckProcessRlimits(t *testing.T) { + for _, tc := range []struct { + name string + rlimits []specs.POSIXRlimit + isErr bool + }{ + {name: "none"}, + { + name: "distinct", + rlimits: []specs.POSIXRlimit{ + {Type: "RLIMIT_NOFILE", Soft: 32, Hard: 64}, + {Type: "RLIMIT_CORE"}, + }, + }, + { + name: "duplicate", + rlimits: []specs.POSIXRlimit{ + {Type: "RLIMIT_NOFILE", Soft: 32, Hard: 64}, + {Type: "RLIMIT_NOFILE", Soft: 48, Hard: 64}, + }, + isErr: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + err := checkProcessRlimits(&specs.Process{Rlimits: tc.rlimits}) + if tc.isErr && err == nil { + t.Fatal("expected error, got nil") + } + if !tc.isErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +}