From 19bd9b33dd8c8f166327f4e1ab18e0cd5afc9180 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <gscrivan@redhat.com>
Date: Mon, 11 Sep 2023 17:04:23 +0200
Subject: [PATCH] libpod: move oom_score_adj clamp to init

commit 8b4a79a744ac3fd176ca4dc0e3ae40f75159e090 introduced
oom_score_adj clamping when the container oom_score_adj value is lower
than the current one in a rootless environment.  Move the check to
init() time so it is performed every time the container starts and not
only when it is created.  It is more robust if the oom_score_adj value
is changed for the current user session.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
---
 libpod/container_internal_common.go | 23 +++++++++++++++++++++++
 pkg/specgen/generate/oci_linux.go   | 29 ++---------------------------
 test/e2e/run_test.go                | 11 +++++++++--
 3 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go
index a8747f8490..ea133073e0 100644
--- a/libpod/container_internal_common.go
+++ b/libpod/container_internal_common.go
@@ -633,6 +633,13 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc
 	nprocSet := false
 	isRootless := rootless.IsRootless()
 	if isRootless {
+		if g.Config.Process != nil && g.Config.Process.OOMScoreAdj != nil {
+			var err error
+			*g.Config.Process.OOMScoreAdj, err = maybeClampOOMScoreAdj(*g.Config.Process.OOMScoreAdj)
+			if err != nil {
+				return nil, nil, err
+			}
+		}
 		for _, rlimit := range c.config.Spec.Process.Rlimits {
 			if rlimit.Type == "RLIMIT_NOFILE" {
 				nofileSet = true
@@ -2938,3 +2945,19 @@ func (c *Container) umask() (uint32, error) {
 	}
 	return uint32(decVal), nil
 }
+
+func maybeClampOOMScoreAdj(oomScoreValue int) (int, error) {
+	v, err := os.ReadFile("/proc/self/oom_score_adj")
+	if err != nil {
+		return oomScoreValue, err
+	}
+	currentValue, err := strconv.Atoi(strings.TrimRight(string(v), "\n"))
+	if err != nil {
+		return oomScoreValue, err
+	}
+	if currentValue > oomScoreValue {
+		logrus.Warnf("Requested oom_score_adj=%d is lower than the current one, changing to %d", oomScoreValue, currentValue)
+		return currentValue, nil
+	}
+	return oomScoreValue, nil
+}
diff --git a/pkg/specgen/generate/oci_linux.go b/pkg/specgen/generate/oci_linux.go
index 78d0bef21e..bd1e65146f 100644
--- a/pkg/specgen/generate/oci_linux.go
+++ b/pkg/specgen/generate/oci_linux.go
@@ -4,9 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"os"
 	"path"
-	"strconv"
 	"strings"
 
 	"github.com/containers/common/libimage"
@@ -18,7 +16,6 @@ import (
 	"github.com/containers/podman/v4/pkg/specgen"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
-	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
 
@@ -81,25 +78,6 @@ func getCgroupPermissions(unmask []string) string {
 	return ro
 }
 
-func maybeClampOOMScoreAdj(oomScoreValue int, isRootless bool) (int, error) {
-	if !isRootless {
-		return oomScoreValue, nil
-	}
-	v, err := os.ReadFile("/proc/self/oom_score_adj")
-	if err != nil {
-		return oomScoreValue, err
-	}
-	currentValue, err := strconv.Atoi(strings.TrimRight(string(v), "\n"))
-	if err != nil {
-		return oomScoreValue, err
-	}
-	if currentValue > oomScoreValue {
-		logrus.Warnf("Requested oom_score_adj=%d is lower than the current one, changing to %d", oomScoreValue, currentValue)
-		return currentValue, nil
-	}
-	return oomScoreValue, nil
-}
-
 // SpecGenToOCI returns the base configuration for the container.
 func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runtime, rtc *config.Config, newImage *libimage.Image, mounts []spec.Mount, pod *libpod.Pod, finalCmd []string, compatibleOptions *libpod.InfraInherit) (*spec.Spec, error) {
 	cgroupPerm := getCgroupPermissions(s.Unmask)
@@ -343,12 +321,9 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 	}
 
 	if s.OOMScoreAdj != nil {
-		score, err := maybeClampOOMScoreAdj(*s.OOMScoreAdj, isRootless)
-		if err != nil {
-			return nil, err
-		}
-		g.SetProcessOOMScoreAdj(score)
+		g.SetProcessOOMScoreAdj(*s.OOMScoreAdj)
 	}
+
 	setProcOpts(s, &g)
 	if s.ReadOnlyFilesystem && !s.ReadWriteTmpfs {
 		setDevOptsReadOnly(&g)
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index 4ebcaeef81..ae8db35d9a 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -648,10 +648,17 @@ USER bin`, BB)
 
 		currentOOMScoreAdj, err := os.ReadFile("/proc/self/oom_score_adj")
 		Expect(err).ToNot(HaveOccurred())
-		session = podmanTest.Podman([]string{"run", "--rm", fedoraMinimal, "cat", "/proc/self/oom_score_adj"})
+		name := "ctr-with-oom-score"
+		session = podmanTest.Podman([]string{"create", "--name", name, fedoraMinimal, "cat", "/proc/self/oom_score_adj"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
-		Expect(session.OutputToString()).To(Equal(strings.TrimRight(string(currentOOMScoreAdj), "\n")))
+
+		for i := 0; i < 2; i++ {
+			session = podmanTest.Podman([]string{"start", "-a", name})
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(Exit(0))
+			Expect(session.OutputToString()).To(Equal(strings.TrimRight(string(currentOOMScoreAdj), "\n")))
+		}
 	})
 
 	It("podman run limits host test", func() {