Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
fixed. (#5403, #5487)

### 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
Expand Down
10 changes: 9 additions & 1 deletion exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
14 changes: 14 additions & 0 deletions libcontainer/configs/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"os/exec"
"slices"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -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
}
7 changes: 7 additions & 0 deletions libcontainer/configs/validate/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func Validate(config *configs.Config) error {
scheduler,
ioPriority,
memoryPolicy,
rlimits,
}
for _, c := range checks {
if err := c(config); err != nil {
Expand Down Expand Up @@ -506,3 +507,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)
}
36 changes: 36 additions & 0 deletions libcontainer/configs/validate/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1077,3 +1077,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)
}
})
}
}
3 changes: 3 additions & 0 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,9 @@ func (c *Container) start(process *Process) (retErr error) {
}

if process.Init {
if err := configs.CheckRlimits(process.Rlimits); err != nil {
return err
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check should be done after container already has init process.

if c.initProcessStartTime != 0 {
return errors.New("container already has init process")
}
Expand Down
37 changes: 37 additions & 0 deletions tests/integration/exec.bats
Original file line number Diff line number Diff line change
Expand Up @@ -424,3 +424,40 @@ EOF
[ "$status" -eq 0 ]
[ "${lines[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]" {
runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
[ "$status" -eq 0 ]

# 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}
]'

runc exec test_busybox true
[ "$status" -eq 0 ]

proc='{"terminal": false, "cwd": "/", "args": ["true"],
"rlimits": [
{"type": "RLIMIT_NOFILE", "soft": 32, "hard": 64},
{"type": "RLIMIT_NOFILE", "soft": 48, "hard": 64}
]}'
runc exec --process <(echo "$proc") test_busybox
[ "$status" -ne 0 ]
[[ "$output" == *"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}
]'

runc run test_busybox
[ "$status" -ne 0 ]
[[ "$output" == *"duplicate rlimit type"* ]]
}
13 changes: 13 additions & 0 deletions utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,19 @@ func (r *runner) checkTerminal(config *specs.Process) error {
return nil
}

// checkProcessRlimits returns an error if spec has duplicate rlimit types.
func checkProcessRlimits(spec *specs.Process) error {
rlimits := make([]configs.Rlimit, 0, len(spec.Rlimits))
for _, r := range spec.Rlimits {
rl, err := createLibContainerRlimit(r)
if err != nil {
return err
}
rlimits = append(rlimits, rl)
}
return configs.CheckRlimits(rlimits)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that we convert just to validate. Lot of garbage to collect.

Can be checked right here without extra allocations:

// 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")
Expand Down
47 changes: 47 additions & 0 deletions utils_linux_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
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,
},
{
name: "unknown type",
rlimits: []specs.POSIXRlimit{{Type: "RLIMIT_BOGUS"}},
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)
}
})
}
}