Allow threads with CAP_SYS_RESOURCE to raise hard rlimits.

PiperOrigin-RevId: 242919489
Change-Id: Ie3267b3bcd8a54b54bc16a6556369a19e843376f

7 files changed, 48 insertions, 25 deletions

diff --git a/pkg/sentry/kernel/fd_map_test.go b/pkg/sentry/kernel/fd_map_test.go
index b49996137..9e76f0a2d 100644
--- a/pkg/sentry/kernel/fd_map_test.go
+++ b/pkg/sentry/kernel/fd_map_test.go
@@ -40,7 +40,7 @@ func newTestFDMap() *FDMap {
 func TestFDMapMany(t *testing.T) {
 	file := filetest.NewTestFile(t)
 	limitSet := limits.NewLimitSet()
-	limitSet.Set(limits.NumberOfFiles, limits.Limit{maxFD, maxFD})
+	limitSet.Set(limits.NumberOfFiles, limits.Limit{maxFD, maxFD}, true /* privileged */)
 
 	f := newTestFDMap()
 	for i := 0; i < maxFD; i++ {
@@ -64,7 +64,7 @@ func TestFDMapMany(t *testing.T) {
 func TestFDMap(t *testing.T) {
 	file := filetest.NewTestFile(t)
 	limitSet := limits.NewLimitSet()
-	limitSet.Set(limits.NumberOfFiles, limits.Limit{1, maxFD})
+	limitSet.Set(limits.NumberOfFiles, limits.Limit{1, maxFD}, true /* privileged */)
 
 	f := newTestFDMap()
 	if _, err := f.NewFDFrom(0, file, FDFlags{}, limitSet); err != nil {
@@ -76,7 +76,7 @@ func TestFDMap(t *testing.T) {
 	}
 
 	largeLimit := limits.Limit{maxFD, maxFD}
-	limitSet.Set(limits.NumberOfFiles, largeLimit)
+	limitSet.Set(limits.NumberOfFiles, largeLimit, true /* privileged */)
 
 	if fd, err := f.NewFDFrom(0, file, FDFlags{}, limitSet); err != nil {
 		t.Fatalf("Adding an FD to a resized map: got %v, want nil", err)
@@ -117,7 +117,7 @@ func TestDescriptorFlags(t *testing.T) {
 	file := filetest.NewTestFile(t)
 	f := newTestFDMap()
 	limitSet := limits.NewLimitSet()
-	limitSet.Set(limits.NumberOfFiles, limits.Limit{maxFD, maxFD})
+	limitSet.Set(limits.NumberOfFiles, limits.Limit{maxFD, maxFD}, true /* privileged */)
 
 	origFlags := FDFlags{CloseOnExec: true}
 
diff --git a/pkg/sentry/limits/limits.go b/pkg/sentry/limits/limits.go
index eeca01876..b0571739f 100644
--- a/pkg/sentry/limits/limits.go
+++ b/pkg/sentry/limits/limits.go
@@ -113,13 +113,17 @@ func (l *LimitSet) SetUnchecked(t LimitType, v Limit) {
 }
 
 // Set assigns value v to resource of LimitType t and returns the old value.
-func (l *LimitSet) Set(t LimitType, v Limit) (Limit, error) {
+// privileged should be true only when either the caller has CAP_SYS_RESOURCE
+// or when creating limits for a new kernel.
+func (l *LimitSet) Set(t LimitType, v Limit, privileged bool) (Limit, error) {
 	l.mu.Lock()
 	defer l.mu.Unlock()
+
 	// If a limit is already set, make sure the new limit doesn't
 	// exceed the previous max limit.
 	if _, ok := l.data[t]; ok {
-		if l.data[t].Max < v.Max {
+		// Unprivileged users can only lower their hard limits.
+		if l.data[t].Max < v.Max && !privileged {
 			return Limit{}, syscall.EPERM
 		}
 		if v.Cur > v.Max {
diff --git a/pkg/sentry/limits/limits_test.go b/pkg/sentry/limits/limits_test.go
index d41f62554..945428163 100644
--- a/pkg/sentry/limits/limits_test.go
+++ b/pkg/sentry/limits/limits_test.go
@@ -20,18 +20,24 @@ import (
 )
 
 func TestSet(t *testing.T) {
-	ls := NewLimitSet()
-	ls.Set(1, Limit{Cur: 50, Max: 50})
-	if _, err := ls.Set(1, Limit{Cur: 20, Max: 50}); err != nil {
-		t.Fatalf("Tried to lower Limit to valid new value: got %v, wanted nil", err)
-	}
-	if _, err := ls.Set(1, Limit{Cur: 20, Max: 60}); err != syscall.EPERM {
-		t.Fatalf("Tried to raise limit.Max to invalid higher value: got %v, wanted syscall.EPERM", err)
-	}
-	if _, err := ls.Set(1, Limit{Cur: 60, Max: 50}); err != syscall.EINVAL {
-		t.Fatalf("Tried to raise limit.Cur to invalid higher value: got %v, wanted syscall.EINVAL", err)
+	testCases := []struct {
+		limit       Limit
+		privileged  bool
+		expectedErr error
+	}{
+		{limit: Limit{Cur: 50, Max: 50}, privileged: false, expectedErr: nil},
+		{limit: Limit{Cur: 20, Max: 50}, privileged: false, expectedErr: nil},
+		{limit: Limit{Cur: 20, Max: 60}, privileged: false, expectedErr: syscall.EPERM},
+		{limit: Limit{Cur: 60, Max: 50}, privileged: false, expectedErr: syscall.EINVAL},
+		{limit: Limit{Cur: 11, Max: 10}, privileged: false, expectedErr: syscall.EINVAL},
+		{limit: Limit{Cur: 20, Max: 60}, privileged: true, expectedErr: nil},
 	}
-	if _, err := ls.Set(1, Limit{Cur: 11, Max: 10}); err != syscall.EINVAL {
-		t.Fatalf("Tried to set new limit with Cur > Max: got %v, wanted syscall.EINVAL", err)
+
+	ls := NewLimitSet()
+	for _, tc := range testCases {
+		if _, err := ls.Set(1, tc.limit, tc.privileged); err != tc.expectedErr {
+			t.Fatalf("Tried to set Limit to %+v and privilege %t: got %v, wanted %v", tc.limit, tc.privileged, err, tc.expectedErr)
+		}
 	}
+
 }
diff --git a/pkg/sentry/limits/linux.go b/pkg/sentry/limits/linux.go
index 295f9c398..e09d0d2fb 100644
--- a/pkg/sentry/limits/linux.go
+++ b/pkg/sentry/limits/linux.go
@@ -95,6 +95,6 @@ func NewLinuxDistroLimitSet() (*LimitSet, error) {
 	// 1,048,576 ought to be enough for anyone.
 	l := ls.Get(ProcessCount)
 	l.Cur = 1 << 20
-	ls.Set(ProcessCount, l)
+	ls.Set(ProcessCount, l, true /* privileged */)
 	return ls, nil
 }
diff --git a/pkg/sentry/mm/mm_test.go b/pkg/sentry/mm/mm_test.go
index e12cb3bd1..ae4fba478 100644
--- a/pkg/sentry/mm/mm_test.go
+++ b/pkg/sentry/mm/mm_test.go
@@ -70,7 +70,7 @@ func TestUsageASUpdates(t *testing.T) {
 
 func TestBrkDataLimitUpdates(t *testing.T) {
 	limitSet := limits.NewLimitSet()
-	limitSet.Set(limits.Data, limits.Limit{}) // zero RLIMIT_DATA
+	limitSet.Set(limits.Data, limits.Limit{}, true /* privileged */) // zero RLIMIT_DATA
 
 	ctx := contexttest.WithLimitSet(contexttest.Context(t), limitSet)
 	mm := testMemoryManager(ctx)
diff --git a/pkg/sentry/syscalls/linux/sys_rlimit.go b/pkg/sentry/syscalls/linux/sys_rlimit.go
index b0b216045..443334693 100644
--- a/pkg/sentry/syscalls/linux/sys_rlimit.go
+++ b/pkg/sentry/syscalls/linux/sys_rlimit.go
@@ -106,7 +106,13 @@ func prlimit64(t *kernel.Task, resource limits.LimitType, newLim *limits.Limit)
 	if _, ok := setableLimits[resource]; !ok {
 		return limits.Limit{}, syserror.EPERM
 	}
-	oldLim, err := t.ThreadGroup().Limits().Set(resource, *newLim)
+
+	// "A privileged process (under Linux: one with the CAP_SYS_RESOURCE
+	// capability in the initial user namespace) may make arbitrary changes
+	// to either limit value."
+	privileged := t.HasCapabilityIn(linux.CAP_SYS_RESOURCE, t.Kernel().RootUserNamespace())
+
+	oldLim, err := t.ThreadGroup().Limits().Set(resource, *newLim, privileged)
 	if err != nil {
 		return limits.Limit{}, err
 	}
diff --git a/test/syscalls/linux/rlimits.cc b/test/syscalls/linux/rlimits.cc
index dc31bad9a..5a6174d99 100644
--- a/test/syscalls/linux/rlimits.cc
+++ b/test/syscalls/linux/rlimits.cc
@@ -25,15 +25,12 @@ namespace {
 
 TEST(RlimitTest, SetRlimitHigher) {
   SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_RESOURCE)));
-  SKIP_IF(!IsRunningOnGvisor());
 
   struct rlimit rl = {};
   EXPECT_THAT(getrlimit(RLIMIT_NOFILE, &rl), SyscallSucceeds());
 
-  // TODO: Even with CAP_SYS_RESOURCE, gVisor does not allow
-  // setting a higher rlimit.
   rl.rlim_max++;
-  EXPECT_THAT(setrlimit(RLIMIT_NOFILE, &rl), SyscallFailsWithErrno(EPERM));
+  EXPECT_THAT(setrlimit(RLIMIT_NOFILE, &rl), SyscallSucceeds());
 }
 
 TEST(RlimitTest, UnprivilegedSetRlimit) {
@@ -56,6 +53,16 @@ TEST(RlimitTest, UnprivilegedSetRlimit) {
   EXPECT_THAT(setrlimit(RLIMIT_NOFILE, &rl), SyscallFailsWithErrno(EPERM));
 }
 
+TEST(RlimitTest, SetSoftRlimitAboveHard) {
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_RESOURCE)));
+
+  struct rlimit rl = {};
+  EXPECT_THAT(getrlimit(RLIMIT_NOFILE, &rl), SyscallSucceeds());
+
+  rl.rlim_cur = rl.rlim_max + 1;
+  EXPECT_THAT(setrlimit(RLIMIT_NOFILE, &rl), SyscallFailsWithErrno(EINVAL));
+}
+
 }  // namespace
 
 }  // namespace testing