From e66ff395b7c2618f58eb36e33e7324897ae54995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Rod=C3=A1k?= Date: Fri, 25 Apr 2025 17:23:59 +0200 Subject: [PATCH] Fix handling of "r_limits" in Podman REST API /libpod/containers/create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JSON decoder correctly cannot decode (overflow) negative values (e.g., `-1`) for fields of type `uint64`, as `-1` is used to represent `max` in `POSIXRlimit`. To handle this, we use `tmpSpecGenerator` to decode the request body. The `tmpSpecGenerator` replaces the `POSIXRlimit` type with a `tmpRlimit` type that uses the `json.Number` type for decoding values. The `tmpRlimit` is then converted into the `POSIXRlimit` type and assigned to the `SpecGenerator`. This approach ensures compatibility with the Podman CLI and remote API, which already handle `-1` by casting it to `uint64` (`uint64(-1)` equals `MaxUint64`) to signify `max`. Fixes: https://issues.redhat.com/browse/RUN-2859 Fixes: https://github.com/containers/podman/issues/24886 Signed-off-by: Jan Rodák --- pkg/api/handlers/libpod/containers_create.go | 97 +++++++++++++++++--- test/apiv2/20-containers.at | 23 +++++ test/system/030-run.bats | 8 ++ 3 files changed, 115 insertions(+), 13 deletions(-) diff --git a/pkg/api/handlers/libpod/containers_create.go b/pkg/api/handlers/libpod/containers_create.go index 6b375fccaa..55bb8a9bd4 100644 --- a/pkg/api/handlers/libpod/containers_create.go +++ b/pkg/api/handlers/libpod/containers_create.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "math" "net/http" "strconv" @@ -18,8 +19,28 @@ import ( "github.com/containers/podman/v5/pkg/specgen/generate" "github.com/containers/podman/v5/pkg/specgenutil" "github.com/containers/storage" + "github.com/opencontainers/runtime-spec/specs-go" ) +// The JSON decoder correctly cannot decode (overflow) negative values (e.g., `-1`) for fields of type `uint64`, +// as `-1` is used to represent `max` in `POSIXRlimit`. To address this, we use `tmpSpecGenerator` to decode the request body. +// The `tmpSpecGenerator` overrides the `POSIXRlimit` type with a `tmpRlimit` type that uses the `json.Number` type for decoding values. +// The `tmpRlimit` is then parsed into the `POSIXRlimit` type and assigned to the `SpecGenerator`. +// This serves as a workaround for the issue (https://github.com/containers/podman/issues/24886). +type tmpSpecGenerator struct { + specgen.SpecGenerator + Rlimits []tmpRlimit `json:"r_limits,omitempty"` +} + +type tmpRlimit struct { + // Type of the rlimit to set + Type string `json:"type"` + // Hard is the hard limit for the specified type + Hard json.Number `json:"hard"` + // Soft is the soft limit for the specified type + Soft json.Number `json:"soft"` +} + // CreateContainer takes a specgenerator and makes a container. It returns // the new container ID on success along with any warnings. func CreateContainer(w http.ResponseWriter, r *http.Request) { @@ -35,25 +56,36 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) { privileged := conf.Containers.Privileged // we have to set the default before we decode to make sure the correct default is set when the field is unset - sg := specgen.SpecGenerator{ - ContainerNetworkConfig: specgen.ContainerNetworkConfig{ - UseImageHosts: &noHosts, - }, - ContainerSecurityConfig: specgen.ContainerSecurityConfig{ - Umask: conf.Containers.Umask, - Privileged: &privileged, - }, - ContainerHealthCheckConfig: specgen.ContainerHealthCheckConfig{ - HealthLogDestination: define.DefaultHealthCheckLocalDestination, - HealthMaxLogCount: define.DefaultHealthMaxLogCount, - HealthMaxLogSize: define.DefaultHealthMaxLogSize, + tmpSg := tmpSpecGenerator{ + SpecGenerator: specgen.SpecGenerator{ + ContainerNetworkConfig: specgen.ContainerNetworkConfig{ + UseImageHosts: &noHosts, + }, + ContainerSecurityConfig: specgen.ContainerSecurityConfig{ + Umask: conf.Containers.Umask, + Privileged: &privileged, + }, + ContainerHealthCheckConfig: specgen.ContainerHealthCheckConfig{ + HealthLogDestination: define.DefaultHealthCheckLocalDestination, + HealthMaxLogCount: define.DefaultHealthMaxLogCount, + HealthMaxLogSize: define.DefaultHealthMaxLogSize, + }, }, } - if err := json.NewDecoder(r.Body).Decode(&sg); err != nil { + if err := json.NewDecoder(r.Body).Decode(&tmpSg); err != nil { utils.Error(w, http.StatusInternalServerError, fmt.Errorf("decode(): %w", err)) return } + + sg := tmpSg.SpecGenerator + rLimits, err := parseRLimits(tmpSg.Rlimits) + if err != nil { + utils.Error(w, http.StatusBadRequest, fmt.Errorf("invalid rlimit: %w", err)) + return + } + sg.Rlimits = rLimits + if sg.Passwd == nil { t := true sg.Passwd = &t @@ -100,3 +132,42 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) { response := entities.ContainerCreateResponse{ID: ctr.ID(), Warnings: warn} utils.WriteJSON(w, http.StatusCreated, response) } + +// parseRLimits parses slice of tmpLimit to slice of specs.POSIXRlimit. +func parseRLimits(rLimits []tmpRlimit) ([]specs.POSIXRlimit, error) { + rl := []specs.POSIXRlimit{} + + // The "soft" and "hard" values are expected to be of type uint64. + // The JSON decoder cannot cast -1 as to uint64. + // We need to convert them to uint64, and handle the special case of -1 + // which indicates an max value. + parseLimitNumber := func(limit json.Number) (uint64, error) { + limitString := limit.String() + if limitString == "-1" { + // uint64(-1) overflow to max uint64 value + return math.MaxUint64, nil + } + return strconv.ParseUint(limitString, 10, 64) + } + + for _, rLimit := range rLimits { + soft, err := parseLimitNumber(rLimit.Soft) + if err != nil { + return nil, fmt.Errorf("invalid value for POSIXRlimit.soft: %w", err) + } + hard, err := parseLimitNumber(rLimit.Hard) + if err != nil { + return nil, fmt.Errorf("invalid value for POSIXRlimit.hard: %w", err) + } + if rLimit.Type == "" { + return nil, fmt.Errorf("invalid value for POSIXRlimit.type: %w", err) + } + + rl = append(rl, specs.POSIXRlimit{ + Type: rLimit.Type, + Soft: soft, + Hard: hard, + }) + } + return rl, nil +} diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index 7b8c5a6bf0..28712fccfe 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -772,6 +772,29 @@ if root; then podman rm -f updateCtr fi + +# test if API support -1 for ulimits https://github.com/containers/podman/issues/24886 + +# Compat API +t POST containers/create \ + Image=$IMAGE \ + HostConfig='{"Ulimits":[{"Name":"memlock","Soft":-1,"Hard":-1}]}' \ + 201 \ + .Id~[0-9a-f]\\{64\\} +cid=$(jq -r '.Id' <<<"$output") + +t DELETE containers/$cid 204 + +# Libpod API +t POST libpod/containers/create \ + Image=$IMAGE \ + r_limits='[{"type":"memlock","soft":-1,"hard":-1}]' \ + 201 \ + .Id~[0-9a-f]\\{64\\} +cid=$(jq -r '.Id' <<<"$output") + +t DELETE containers/$cid 204 + rm -rf $TMPD podman container rm -fa diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 9853abc494..bed44e6850 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -1309,6 +1309,14 @@ EOF fi fi + ctr="c-h-$(safename)" + run_podman run -d --name $ctr --ulimit core=-1:-1 $IMAGE /home/podman/pause + + run_podman inspect $ctr --format "{{.HostConfig.Ulimits}}" + assert "$output" =~ "RLIMIT_CORE -1 -1" "ulimit core is not set to unlimited" + + run_podman rm -f $ctr + run_podman run --ulimit core=-1:-1 --rm $IMAGE grep core /proc/self/limits assert "$output" =~ " ${max} * ${max} * bytes"