From da2fd657ab109d71e68ae46799eec54d4f8e21a6 Mon Sep 17 00:00:00 2001
From: Simon Kaegi <simon.kaegi@gmail.com>
Date: Mon, 14 Dec 2020 12:01:04 -0500
Subject: [PATCH] Add bounds on max oom_score_adj value for AdjustOOMScore

oom_score_adj must be in the range -1000 to 1000. In AdjustOOMScore if containerd's score is already at the maximum value we should set that value for the shim instead of trying to set 1001 which is invalid.

Signed-off-by: Simon Kaegi <simon_kaegi@ca.ibm.com>
---
 container_linux_test.go          | 4 ++++
 runtime/v1/shim/client/client.go | 4 ++++
 runtime/v2/shim/util_unix.go     | 4 ++++
 sys/oom_unix.go                  | 8 ++++++--
 sys/oom_windows.go               | 5 +++++
 5 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/container_linux_test.go b/container_linux_test.go
index ec02042ad..43c1d6904 100644
--- a/container_linux_test.go
+++ b/container_linux_test.go
@@ -2043,6 +2043,10 @@ func TestShimOOMScore(t *testing.T) {
 	}
 
 	expectedScore := containerdScore + 1
+	if expectedScore > sys.OOMScoreAdjMax {
+		expectedScore = sys.OOMScoreAdjMax
+	}
+
 	// find the shim's pid
 	if cgroups.Mode() == cgroups.Unified {
 		processes, err := cg2.Procs(false)
diff --git a/runtime/v1/shim/client/client.go b/runtime/v1/shim/client/client.go
index e35dafec3..9211c0052 100644
--- a/runtime/v1/shim/client/client.go
+++ b/runtime/v1/shim/client/client.go
@@ -174,6 +174,7 @@ func eaddrinuse(err error) bool {
 
 // setupOOMScore gets containerd's oom score and adds +1 to it
 // to ensure a shim has a lower* score than the daemons
+// if not already at the maximum OOM Score
 func setupOOMScore(shimPid int) error {
 	pid := os.Getpid()
 	score, err := sys.GetOOMScoreAdj(pid)
@@ -181,6 +182,9 @@ func setupOOMScore(shimPid int) error {
 		return errors.Wrap(err, "get daemon OOM score")
 	}
 	shimScore := score + 1
+	if shimScore > sys.OOMScoreAdjMax {
+		shimScore = sys.OOMScoreAdjMax
+	}
 	if err := sys.SetOOMScore(shimPid, shimScore); err != nil {
 		return errors.Wrap(err, "set shim OOM score")
 	}
diff --git a/runtime/v2/shim/util_unix.go b/runtime/v2/shim/util_unix.go
index 2b0d0ada3..9fb7cc573 100644
--- a/runtime/v2/shim/util_unix.go
+++ b/runtime/v2/shim/util_unix.go
@@ -53,6 +53,7 @@ func SetScore(pid int) error {
 
 // AdjustOOMScore sets the OOM score for the process to the parents OOM score +1
 // to ensure that they parent has a lower* score than the shim
+// if not already at the maximum OOM Score
 func AdjustOOMScore(pid int) error {
 	parent := os.Getppid()
 	score, err := sys.GetOOMScoreAdj(parent)
@@ -60,6 +61,9 @@ func AdjustOOMScore(pid int) error {
 		return errors.Wrap(err, "get parent OOM score")
 	}
 	shimScore := score + 1
+	if shimScore > sys.OOMScoreAdjMax {
+		shimScore = sys.OOMScoreAdjMax
+	}
 	if err := sys.SetOOMScore(pid, shimScore); err != nil {
 		return errors.Wrap(err, "set shim OOM score")
 	}
diff --git a/sys/oom_unix.go b/sys/oom_unix.go
index d49d5bc8d..c381e1a7e 100644
--- a/sys/oom_unix.go
+++ b/sys/oom_unix.go
@@ -26,8 +26,12 @@ import (
 	"strings"
 )
 
-// OOMScoreMaxKillable is the maximum score keeping the process killable by the oom killer
-const OOMScoreMaxKillable = -999
+const (
+	// OOMScoreMaxKillable is the maximum score keeping the process killable by the oom killer
+	OOMScoreMaxKillable = -999
+	// OOMScoreAdjMax is from OOM_SCORE_ADJ_MAX https://github.com/torvalds/linux/blob/master/include/uapi/linux/oom.h
+	OOMScoreAdjMax = 1000
+)
 
 // SetOOMScore sets the oom score for the provided pid
 func SetOOMScore(pid, score int) error {
diff --git a/sys/oom_windows.go b/sys/oom_windows.go
index a917ba635..215c171f6 100644
--- a/sys/oom_windows.go
+++ b/sys/oom_windows.go
@@ -16,6 +16,11 @@
 
 package sys
 
+const (
+	// OOMScoreAdjMax is not implemented on Windows
+	OOMScoreAdjMax = 0
+)
+
 // SetOOMScore sets the oom score for the process
 //
 // Not implemented on Windows