sys: add boundary checks to SetOOMScore()

thaJeztah · thaJeztah · commit 44240116aa8b · 2021-04-07T19:16:55.000+02:00
Produce a more user-friendly error in the odd case we would try to set a
score out of range

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
diff --git a/sys/oom_linux.go b/sys/oom_linux.go
@@ -30,12 +30,17 @@ import (
 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
+	// OOMScoreAdjMin is from OOM_SCORE_ADJ_MIN https://github.com/torvalds/linux/blob/v5.10/include/uapi/linux/oom.h#L9
+	OOMScoreAdjMin = -1000
+	// OOMScoreAdjMax is from OOM_SCORE_ADJ_MAX https://github.com/torvalds/linux/blob/v5.10/include/uapi/linux/oom.h#L10
 	OOMScoreAdjMax = 1000
 )
 
 // SetOOMScore sets the oom score for the provided pid
 func SetOOMScore(pid, score int) error {
+	if score > OOMScoreAdjMax || score < OOMScoreAdjMin {
+		return fmt.Errorf("value out of range (%d): OOM score must be between %d and %d", score, OOMScoreAdjMin, OOMScoreAdjMax)
+	}
 	path := fmt.Sprintf("/proc/%d/oom_score_adj", pid)
 	f, err := os.OpenFile(path, os.O_WRONLY, 0)
 	if err != nil {
diff --git a/sys/oom_linux_test.go b/sys/oom_linux_test.go
@@ -18,6 +18,7 @@ package sys
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"os/exec"
 	"testing"
@@ -57,6 +58,28 @@ func TestSetNegativeOomScoreAdjustmentWhenUnprivilegedHasNoEffect(t *testing.T)
 	assert.Check(t, is.Equal(adjustment, initial))
 }
 
+func TestSetOOMScoreBoundaries(t *testing.T) {
+	err := SetOOMScore(0, OOMScoreAdjMax+1)
+	assert.ErrorContains(t, err, fmt.Sprintf("value out of range (%d): OOM score must be between", OOMScoreAdjMax+1))
+
+	err = SetOOMScore(0, OOMScoreAdjMin-1)
+	assert.ErrorContains(t, err, fmt.Sprintf("value out of range (%d): OOM score must be between", OOMScoreAdjMin-1))
+
+	_, adjustment, err := adjustOom(OOMScoreAdjMax)
+	assert.NilError(t, err)
+	assert.Check(t, is.Equal(adjustment, OOMScoreAdjMax))
+
+	score, err := GetOOMScoreAdj(os.Getpid())
+	assert.NilError(t, err)
+	if score == 0 || score == OOMScoreAdjMin {
+		// we won't be able to set the score lower than the parent process,
+		// so only test if parent process does not have a oom-score-adj
+		_, adjustment, err = adjustOom(OOMScoreAdjMin)
+		assert.NilError(t, err)
+		assert.Check(t, is.Equal(adjustment, OOMScoreAdjMin))
+	}
+}
+
 func adjustOom(adjustment int) (int, int, error) {
 	cmd := exec.Command("sleep", "100")
 	if err := cmd.Start(); err != nil {
