Fix process/config properties merging

CHANGELOG.md

@@ -10,6 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * `configs.CommandHook` struct has changed, Command is now a pointer.
    Also, `configs.NewCommandHook` now accepts a `*Command`. (#4325)
 
+### Fixed
+ * `runc exec -p` no longer ignores specified `ioPriority` and `scheduler`
+   settings. Similarly, libcontainer's `Container.Start` and `Container.Run`
+   methods no longer ignore `Process.IOPriority` and `Process.Scheduler`
+   settings. (#4585)
+
 ## [1.2.0] - 2024-10-22
 
 > できるときにできることをやるんだ。それが今だ。

libcontainer/capabilities/capabilities.go

@@ -45,6 +45,9 @@ func KnownCapabilities() []string {
 // printing a warning instead.
 func New(capConfig *configs.Capabilities) (*Caps, error) {
 	var c Caps
+	if capConfig == nil {
+		return &c, nil
+	}
 
 	_, err := capMap()
 	if err != nil {
@@ -101,13 +104,19 @@ type Caps struct {
 
 // ApplyBoundingSet sets the capability bounding set to those specified in the whitelist.
 func (c *Caps) ApplyBoundingSet() error {
+	if c.pid == nil {
+		return nil
+	}
 	c.pid.Clear(capability.BOUNDING)
 	c.pid.Set(capability.BOUNDING, c.caps[capability.BOUNDING]...)
 	return c.pid.Apply(capability.BOUNDING)
 }
 
 // Apply sets all the capabilities for the current process in the config.
 func (c *Caps) ApplyCaps() error {
+	if c.pid == nil {
+		return nil
+	}
 	c.pid.Clear(capability.CAPS | capability.BOUNDS)
 	for _, g := range []capability.CapType{
 		capability.EFFECTIVE,

libcontainer/container_linux.go

@@ -681,26 +681,35 @@ func (c *Container) newSetnsProcess(p *Process, cmd *exec.Cmd, comm *processComm
 }
 
 func (c *Container) newInitConfig(process *Process) *initConfig {
+	// Set initial properties. For those properties that exist
+	// both in the container config and the process, use the ones
+	// from the container config first, and override them later.
 	cfg := &initConfig{
 		Config:           c.config,
 		Args:             process.Args,
 		Env:              process.Env,
 		User:             process.User,
 		AdditionalGroups: process.AdditionalGroups,
 		Cwd:              process.Cwd,
-		Capabilities:     process.Capabilities,
+		Capabilities:     c.config.Capabilities,
 		PassedFilesCount: len(process.ExtraFiles),
 		ContainerID:      c.ID(),
 		NoNewPrivileges:  c.config.NoNewPrivileges,
-		RootlessEUID:     c.config.RootlessEUID,
-		RootlessCgroups:  c.config.RootlessCgroups,
 		AppArmorProfile:  c.config.AppArmorProfile,
 		ProcessLabel:     c.config.ProcessLabel,
 		Rlimits:          c.config.Rlimits,
+		IOPriority:       c.config.IOPriority,
+		Scheduler:        c.config.Scheduler,
 		CreateConsole:    process.ConsoleSocket != nil,
 		ConsoleWidth:     process.ConsoleWidth,
 		ConsoleHeight:    process.ConsoleHeight,
 	}
+
+	// Overwrite config properties with ones from process.
+
+	if process.Capabilities != nil {
+		cfg.Capabilities = process.Capabilities
+	}
 	if process.NoNewPrivileges != nil {
 		cfg.NoNewPrivileges = *process.NoNewPrivileges
 	}
@@ -713,6 +722,15 @@ func (c *Container) newInitConfig(process *Process) *initConfig {
 	if len(process.Rlimits) > 0 {
 		cfg.Rlimits = process.Rlimits
 	}
+	if process.IOPriority != nil {
+		cfg.IOPriority = process.IOPriority
+	}
+	if process.Scheduler != nil {
+		cfg.Scheduler = process.Scheduler
+	}
+
+	// Set misc properties.
+
 	if cgroups.IsCgroup2UnifiedMode() {
 		cfg.Cgroup2Path = c.cgroupManager.Path("")
 	}

libcontainer/init_linux.go

@@ -48,29 +48,53 @@ type network struct {
 	TempVethPeerName string `json:"temp_veth_peer_name"`
 }
 
-// initConfig is used for transferring parameters from Exec() to Init()
+// initConfig is used for transferring parameters from Exec() to Init().
+// It contains:
+//   - original container config;
+//   - some [Process] properties;
+//   - set of properties merged from the container config ([configs.Config])
+//     and the process ([Process]);
+//   - some properties that come from the container.
+//
+// When adding new fields, please make sure they go into the relevant section.
 type initConfig struct {
-	Args             []string              `json:"args"`
-	Env              []string              `json:"env"`
-	Cwd              string                `json:"cwd"`
-	Capabilities     *configs.Capabilities `json:"capabilities"`
-	ProcessLabel     string                `json:"process_label"`
-	AppArmorProfile  string                `json:"apparmor_profile"`
-	NoNewPrivileges  bool                  `json:"no_new_privileges"`
-	User             string                `json:"user"`
-	AdditionalGroups []string              `json:"additional_groups"`
-	Config           *configs.Config       `json:"config"`
-	Networks         []*network            `json:"network"`
-	PassedFilesCount int                   `json:"passed_files_count"`
-	ContainerID      string                `json:"containerid"`
-	Rlimits          []configs.Rlimit      `json:"rlimits"`
-	CreateConsole    bool                  `json:"create_console"`
-	ConsoleWidth     uint16                `json:"console_width"`
-	ConsoleHeight    uint16                `json:"console_height"`
-	RootlessEUID     bool                  `json:"rootless_euid,omitempty"`
-	RootlessCgroups  bool                  `json:"rootless_cgroups,omitempty"`
-	SpecState        *specs.State          `json:"spec_state,omitempty"`
-	Cgroup2Path      string                `json:"cgroup2_path,omitempty"`
+	// Config is the original container config.
+	Config *configs.Config `json:"config"`
+
+	// Properties that are unique to and come from [Process].
+
+	Args             []string `json:"args"`
+	Env              []string `json:"env"`
+	User             string   `json:"user"`
+	AdditionalGroups []string `json:"additional_groups"`
+	Cwd              string   `json:"cwd"`
+	CreateConsole    bool     `json:"create_console"`
+	ConsoleWidth     uint16   `json:"console_width"`
+	ConsoleHeight    uint16   `json:"console_height"`
+	PassedFilesCount int      `json:"passed_files_count"`
+
+	// Properties that exists both in the container config and the process,
+	// as merged by [Container.newInitConfig] (process properties has preference).
+
+	AppArmorProfile string                `json:"apparmor_profile"`
+	Capabilities    *configs.Capabilities `json:"capabilities"`
+	NoNewPrivileges bool                  `json:"no_new_privileges"`
+	ProcessLabel    string                `json:"process_label"`
+	Rlimits         []configs.Rlimit      `json:"rlimits"`
+	IOPriority      *configs.IOPriority   `json:"io_priority,omitempty"`
+	Scheduler       *configs.Scheduler    `json:"scheduler,omitempty"`
+
+	// Miscellaneous properties, filled in by [Container.newInitConfig]
+	// unless documented otherwise.
+
+	ContainerID string `json:"containerid"`
+	Cgroup2Path string `json:"cgroup2_path,omitempty"`
+
+	// Networks is filled in from container config by [initProcess.createNetworkInterfaces].
+	Networks []*network `json:"network"`
+
+	// SpecState is filled in by [initProcess.Start].
+	SpecState *specs.State `json:"spec_state,omitempty"`
 }
 
 // Init is part of "runc init" implementation.
@@ -302,13 +326,7 @@ func finalizeNamespace(config *initConfig, addHome bool) error {
 		}
 	}
 
-	caps := &configs.Capabilities{}
-	if config.Capabilities != nil {
-		caps = config.Capabilities
-	} else if config.Config.Capabilities != nil {
-		caps = config.Config.Capabilities
-	}
-	w, err := capabilities.New(caps)
+	w, err := capabilities.New(config.Capabilities)
 	if err != nil {
 		return err
 	}
@@ -471,7 +489,7 @@ func setupUser(config *initConfig, addHome bool) error {
 		}
 	}
 
-	if config.RootlessEUID {
+	if config.Config.RootlessEUID {
 		// We cannot set any additional groups in a rootless container and thus
 		// we bail if the user asked us to do so. TODO: We currently can't do
 		// this check earlier, but if libcontainer.Process.User was typesafe
@@ -499,7 +517,7 @@ func setupUser(config *initConfig, addHome bool) error {
 	// There's nothing we can do about /etc/group entries, so we silently
 	// ignore setting groups here (since the user didn't explicitly ask us to
 	// set the group).
-	allowSupGroups := !config.RootlessEUID && string(bytes.TrimSpace(setgroups)) != "deny"
+	allowSupGroups := !config.Config.RootlessEUID && string(bytes.TrimSpace(setgroups)) != "deny"
 
 	if allowSupGroups {
 		suppGroups := append(execUser.Sgids, addGroups...)
@@ -639,7 +657,7 @@ func setupRlimits(limits []configs.Rlimit, pid int) error {
 	return nil
 }
 
-func setupScheduler(config *configs.Config) error {
+func setupScheduler(config *initConfig) error {
 	if config.Scheduler == nil {
 		return nil
 	}
@@ -648,15 +666,15 @@ func setupScheduler(config *configs.Config) error {
 		return err
 	}
 	if err := unix.SchedSetAttr(0, attr, 0); err != nil {
-		if errors.Is(err, unix.EPERM) && config.Cgroups.CpusetCpus != "" {
+		if errors.Is(err, unix.EPERM) && config.Config.Cgroups.CpusetCpus != "" {
 			return errors.New("process scheduler can't be used together with AllowedCPUs")
 		}
 		return fmt.Errorf("error setting scheduler: %w", err)
 	}
 	return nil
 }
 
-func setupIOPriority(config *configs.Config) error {
+func setupIOPriority(config *initConfig) error {
 	const ioprioWhoPgrp = 1
 
 	ioprio := config.IOPriority

libcontainer/process.go

@@ -17,8 +17,11 @@ type processOperations interface {
 	pid() int
 }
 
-// Process specifies the configuration and IO for a process inside
-// a container.
+// Process defines the configuration and IO for a process inside a container.
+//
+// Note that some Process properties are also present in container configuration
+// ([configs.Config]). In all such cases, Process properties take precedence
+// over container configuration ones.
 type Process struct {
 	// The command to be run followed by any arguments.
 	Args []string
@@ -34,44 +37,54 @@ type Process struct {
 	// in addition to those that the user belongs to.
 	AdditionalGroups []string
 
-	// Cwd will change the processes current working directory inside the container's rootfs.
+	// Cwd will change the process's current working directory inside the container's rootfs.
 	Cwd string
 
-	// Stdin is a pointer to a reader which provides the standard input stream.
+	// Stdin is a reader which provides the standard input stream.
 	Stdin io.Reader
 
-	// Stdout is a pointer to a writer which receives the standard output stream.
+	// Stdout is a writer which receives the standard output stream.
 	Stdout io.Writer
 
-	// Stderr is a pointer to a writer which receives the standard error stream.
+	// Stderr is a writer which receives the standard error stream.
 	Stderr io.Writer
 
-	// ExtraFiles specifies additional open files to be inherited by the container
+	// ExtraFiles specifies additional open files to be inherited by the process.
 	ExtraFiles []*os.File
 
-	// open handles to cloned binaries -- see dmz.CloneSelfExe for more details
+	// Open handles to cloned binaries -- see dmz.CloneSelfExe for more details.
 	clonedExes []*os.File
 
-	// Initial sizings for the console
+	// Initial size for the console.
 	ConsoleWidth  uint16
 	ConsoleHeight uint16
 
-	// Capabilities specify the capabilities to keep when executing the process inside the container
-	// All capabilities not specified will be dropped from the processes capability mask
+	// Capabilities specify the capabilities to keep when executing the process.
+	// All capabilities not specified will be dropped from the processes capability mask.
+	//
+	// If not nil, takes precedence over container's [configs.Config.Capabilities].
 	Capabilities *configs.Capabilities
 
 	// AppArmorProfile specifies the profile to apply to the process and is
-	// changed at the time the process is execed
+	// changed at the time the process is executed.
+	//
+	// If not empty, takes precedence over container's [configs.Config.AppArmorProfile].
 	AppArmorProfile string
 
-	// Label specifies the label to apply to the process.  It is commonly used by selinux
+	// Label specifies the label to apply to the process. It is commonly used by selinux.
+	//
+	// If not empty, takes precedence over container's [configs.Config.ProcessLabel].
 	Label string
 
 	// NoNewPrivileges controls whether processes can gain additional privileges.
+	//
+	// If not nil, takes precedence over container's [configs.Config.NoNewPrivileges].
 	NoNewPrivileges *bool
 
-	// Rlimits specifies the resource limits, such as max open files, to set in the container
-	// If Rlimits are not set, the container will inherit rlimits from the parent process
+	// Rlimits specifies the resource limits, such as max open files, to set for the process.
+	// If unset, the process will inherit rlimits from the parent process.
+	//
+	// If not empty, takes precedence over container's [configs.Config.Rlimit].
 	Rlimits []configs.Rlimit
 
 	// ConsoleSocket provides the masterfd console.
@@ -99,8 +112,14 @@ type Process struct {
 	// For cgroup v2, the only key allowed is "".
 	SubCgroupPaths map[string]string
 
+	// Scheduler represents the scheduling attributes for a process.
+	//
+	// If not empty, takes precedence over container's [configs.Config.Scheduler].
 	Scheduler *configs.Scheduler
 
+	// IOPriority is a process I/O priority.
+	//
+	// If not empty, takes precedence over container's [configs.Config.IOPriority].
 	IOPriority *configs.IOPriority
 }
 

libcontainer/rootfs_linux.go

@@ -106,7 +106,7 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) {
 		root:            config.Rootfs,
 		label:           config.MountLabel,
 		cgroup2Path:     iConfig.Cgroup2Path,
-		rootlessCgroups: iConfig.RootlessCgroups,
+		rootlessCgroups: config.RootlessCgroups,
 		cgroupns:        config.Namespaces.Contains(configs.NEWCGROUP),
 	}
 	for _, m := range config.Mounts {

libcontainer/setns_init_linux.go

@@ -72,11 +72,11 @@ func (l *linuxSetnsInit) Init() error {
 		unix.Umask(int(*l.config.Config.Umask))
 	}
 
-	if err := setupScheduler(l.config.Config); err != nil {
+	if err := setupScheduler(l.config); err != nil {
 		return err
 	}
 
-	if err := setupIOPriority(l.config.Config); err != nil {
+	if err := setupIOPriority(l.config); err != nil {
 		return err
 	}
 	// Tell our parent that we're ready to exec. This must be done before the

libcontainer/standard_init_linux.go

@@ -156,11 +156,11 @@ func (l *linuxStandardInit) Init() error {
 		}
 	}
 
-	if err := setupScheduler(l.config.Config); err != nil {
+	if err := setupScheduler(l.config); err != nil {
 		return err
 	}
 
-	if err := setupIOPriority(l.config.Config); err != nil {
+	if err := setupIOPriority(l.config); err != nil {
 		return err
 	}
 

tests/integration/ioprio.bats

@@ -20,11 +20,25 @@ function teardown() {
 	# Check the init process.
 	runc exec test_ioprio ionice -p 1
 	[ "$status" -eq 0 ]
-	[[ "$output" = *'best-effort: prio 4'* ]]
+	[ "${lines[0]}" = 'best-effort: prio 4' ]
 
-	# Check the process made from the exec command.
+	# Check an exec process, which should derive ioprio from config.json.
 	runc exec test_ioprio ionice
 	[ "$status" -eq 0 ]
-
-	[[ "$output" = *'best-effort: prio 4'* ]]
+	[ "${lines[0]}" = 'best-effort: prio 4' ]
+
+	# Check an exec with a priority taken from process.json,
+	# which should override the ioprio in config.json.
+	proc='
+{
+	"terminal": false,
+	"ioPriority": {
+		"class": "IOPRIO_CLASS_IDLE"
+	},
+	"args": [ "/usr/bin/ionice" ],
+	"cwd": "/"
+}'
+	runc exec --process <(echo "$proc") test_ioprio
+	[ "$status" -eq 0 ]
+	[ "${lines[0]}" = 'idle' ]
 }