Bound bitmap list parsing by its declared size

pkg/bitmap/format.go

@@ -106,13 +106,13 @@ func parseToken(token string) (start, end uint32, err error) {
 // of indices, and ranges of set bits may be abbreviated. Examples: "0,2,4",
 // "0,3-7,10", "0-10". Input after the first newline or null byte is discarded.
 //
-// sizeHint sets the initial size of the bitmap, which may prevent reallocation
-// when growing the bitmap during parsing. Ideally sizeHint should be at least
-// as large as the bitmap represented by input, but this is not required.
+// bitLimit is the exclusive upper bound on set bit indices. Indices and range
+// endpoints at or above bitLimit are rejected before any bits in that token are
+// added. An empty input is valid even when bitLimit is zero.
 //
 // Inverse of FormatList.
-func ParseList(input string, sizeHint uint32) (*Bitmap, error) {
-	b := New(sizeHint)
+func ParseList(input string, bitLimit uint32) (*Bitmap, error) {
+	b := New(bitLimit)
 
 	if termIdx := strings.IndexAny(input, "\n\000"); termIdx != -1 {
 		input = input[:termIdx]
@@ -129,6 +129,9 @@ func ParseList(input string, sizeHint uint32) (*Bitmap, error) {
 		if err != nil {
 			return nil, err
 		}
+		if end >= bitLimit {
+			return nil, fmt.Errorf("bit %d is outside bitmap limit %d", end, bitLimit)
+		}
 		for i := start; i <= end; i++ {
 			b.Add(i)
 		}

pkg/bitmap/format_test.go

@@ -95,3 +95,40 @@ func TestParse(t *testing.T) {
 		})
 	}
 }
+
+// Keep invalid indices small so a bounds regression fails without causing a
+// large allocation or overflowing the parser's range counter.
+func TestParseBounds(t *testing.T) {
+	for _, test := range []struct {
+		name      string
+		input     string
+		limit     uint32
+		want      []uint32
+		wantError bool
+	}{
+		{name: "empty", input: "", limit: 0, want: []uint32{}},
+		{name: "zero_limit", input: "0", limit: 0, wantError: true},
+		{name: "last_bit", input: "63", limit: 64, want: []uint32{63}},
+		{name: "at_limit", input: "64", limit: 64, wantError: true},
+		{name: "range_at_limit", input: "0-64", limit: 64, wantError: true},
+		{name: "next_word", input: "64", limit: 65, want: []uint32{64}},
+		{name: "unaligned_limit", input: "64-65", limit: 65, wantError: true},
+		{name: "later_token", input: "1,65", limit: 65, wantError: true},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			got, err := ParseList(test.input, test.limit)
+			if (err != nil) != test.wantError {
+				t.Fatalf("ParseList(%q, %d) error = %v, want error: %t", test.input, test.limit, err, test.wantError)
+			}
+			if test.wantError {
+				if got != nil {
+					t.Fatalf("ParseList(%q, %d) returned a partial bitmap on error", test.input, test.limit)
+				}
+				return
+			}
+			if !slices.Equal(got.ToSlice(), test.want) {
+				t.Errorf("ParseList(%q, %d) = %v, want %v", test.input, test.limit, got.ToSlice(), test.want)
+			}
+		})
+	}
+}

pkg/sentry/fsimpl/cgroup2fs/cpuset.go

@@ -123,10 +123,6 @@ func (cc *cpusetCpus) Write(ctx context.Context, _ *vfs.FileDescription, src use
 	if err != nil {
 		return 0, linuxerr.EINVAL
 	}
-	if got, want := b.Maximum(), maxCpus; got > want {
-		return 0, linuxerr.EINVAL
-	}
-
 	cc.cs.mu.Lock()
 	defer cc.cs.mu.Unlock()
 	cc.cs.cpus = b
@@ -173,10 +169,6 @@ func (cm *cpusetMems) Write(ctx context.Context, _ *vfs.FileDescription, src use
 	if err != nil {
 		return 0, linuxerr.EINVAL
 	}
-	if got, want := b.Maximum(), maxMems; got > want {
-		return 0, linuxerr.EINVAL
-	}
-
 	cm.cs.mu.Lock()
 	defer cm.cs.mu.Unlock()
 	cm.cs.mems = b

pkg/sentry/fsimpl/cgroupfs/cpuset.go

@@ -134,11 +134,6 @@ func (d *cpusData) WriteBackground(ctx context.Context, src usermem.IOSequence)
 		return 0, linuxerr.EINVAL
 	}
 
-	if got, want := b.Maximum(), d.c.maxCpus; got > want {
-		log.Warningf("cgroupfs cpuset controller: Attempted to specify cpuset.cpus beyond highest available cpu: got %d, want %d", got, want)
-		return 0, linuxerr.EINVAL
-	}
-
 	d.c.mu.Lock()
 	defer d.c.mu.Unlock()
 	d.c.cpus = b
@@ -188,11 +183,6 @@ func (d *memsData) WriteBackground(ctx context.Context, src usermem.IOSequence)
 		return 0, linuxerr.EINVAL
 	}
 
-	if got, want := b.Maximum(), d.c.maxMems; got > want {
-		log.Warningf("cgroupfs cpuset controller: Attempted to specify cpuset.mems beyond highest available node: got %d, want %d", got, want)
-		return 0, linuxerr.EINVAL
-	}
-
 	d.c.mu.Lock()
 	defer d.c.mu.Unlock()
 	d.c.mems = b

test/syscalls/linux/cgroup2.cc

@@ -774,6 +774,38 @@ TEST_F(Cgroup2Test, SubtreeControlPids) {
               IsPosixErrorOkAndHolds(Not(HasSubstr("pids"))));
 }
 
+TEST_F(Cgroup2Test, CpusetRejectsImpossibleCpu) {
+  const std::string controllers =
+      ASSERT_NO_ERRNO_AND_VALUE(c().ReadControlFile("cgroup.controllers"));
+  SKIP_IF(!absl::StrContains(controllers, "cpuset"));
+
+  const std::string possible =
+      std::string(absl::StripAsciiWhitespace(ASSERT_NO_ERRNO_AND_VALUE(
+          GetContents("/sys/devices/system/cpu/possible"))));
+  // CPU IDs can be sparse or offline. Use the last possible ID, not the number
+  // of online CPUs, to choose a bit outside the system's possible mask.
+  const size_t last = possible.find_last_of(",-");
+  const uint32_t max_cpu = ASSERT_NO_ERRNO_AND_VALUE(Atoi<uint32_t>(
+      possible.substr(last == std::string::npos ? 0 : last + 1)));
+  // Keep the range small even if a regression accepts it.
+  SKIP_IF(max_cpu >= 4096);
+  const std::string impossible = absl::StrCat(max_cpu + 1);
+  const std::string before =
+      ASSERT_NO_ERRNO_AND_VALUE(c().ReadControlFile("cpuset.cpus"));
+  ASSERT_NO_ERRNO(c().WriteControlFile("cpuset.cpus", before));
+  RecordProperty("possible_cpus", possible);
+  for (const auto& [name, value] :
+       {std::pair{"singleton", impossible},
+        std::pair{"range", absl::StrCat("0-", impossible)}}) {
+    SCOPED_TRACE(value);
+    const PosixError error = c().WriteControlFile("cpuset.cpus", value);
+    RecordProperty(absl::StrCat(name, "_errno"), error.errno_value());
+    EXPECT_FALSE(error.ok()) << "Accepted an impossible CPU";
+    EXPECT_THAT(c().ReadControlFile("cpuset.cpus"),
+                IsPosixErrorOkAndHolds(before));
+  }
+}
+
 TEST_F(Cgroup2Test, PidsEnforcement) {
   std::string controllers =
       ASSERT_NO_ERRNO_AND_VALUE(c().ReadControlFile("cgroup.controllers"));