Change --experimental_sandbox_memory_limit to take a number in mebibytes, renaming it to --experimental_sandbox_memory_limit_mb.

Also allows using HOST_RAM.

An int value can only handle up to 2GB, which is not enough.

PiperOrigin-RevId: 509171311
Change-Id: I8e45c0309b72b498e87b707fd1a4143c45a91a93

Author: larsrc-google

src/main/java/com/google/devtools/build/lib/sandbox/BUILD

@@ -35,6 +35,7 @@ java_library(
     deps = [
         "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
         "//src/main/java/com/google/devtools/build/lib/util",
+        "//src/main/java/com/google/devtools/build/lib/util:ram_resource_converter",
         "//src/main/java/com/google/devtools/build/lib/util:resource_converter",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",

src/main/java/com/google/devtools/build/lib/sandbox/CgroupsInfo.java

@@ -291,7 +291,7 @@ public static CgroupsInfo getInstance() throws IOException {
   /**
    * Creates a cgroups directory with the given memory limit.
    *
-   * @param memoryLimit Memory limit in bytes.
+   * @param memoryLimit Memory limit in megabytes (MiB).
    * @param dirName Base name of the directory created. In cgroups v2, <code>.scope</code> gets
    *     appended.
    */
@@ -304,13 +304,13 @@ public String createMemoryLimitCgroupDir(String dirName, int memoryLimit) throws
       // In cgroups v2, we need to propagate the controllers into new subdirs.
       Files.asCharSink(new File(cgroupsDir, "memory.oom.group"), UTF_8).write("1\n");
       Files.asCharSink(new File(cgroupsDir, "memory.max"), UTF_8)
-          .write(Integer.toString(memoryLimit));
+          .write(Long.toString(memoryLimit * 1024L * 1024L));
     } else {
       cgroupsDir = new File(getBlazeDir(), dirName);
       cgroupsDir.mkdirs();
       cgroupsDir.deleteOnExit();
       Files.asCharSink(new File(cgroupsDir, "memory.limit_in_bytes"), UTF_8)
-          .write(Integer.toString(memoryLimit));
+          .write(Long.toString(memoryLimit * 1024L * 1024L));
     }
     return cgroupsDir.toString();
   }

src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java

@@ -331,13 +331,13 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
             .setUseDebugMode(sandboxOptions.sandboxDebug)
             .setKillDelay(timeoutKillDelay);
 
-    if (sandboxOptions.memoryLimit > 0) {
+    if (sandboxOptions.memoryLimitMb > 0) {
       CgroupsInfo cgroupsInfo = CgroupsInfo.getInstance();
       // We put the sandbox inside a unique subdirectory using the context's ID. This ID is
       // unique per spawn run by this spawn runner.
       cgroupsDir =
           cgroupsInfo.createMemoryLimitCgroupDir(
-              "sandbox_" + context.getId(), sandboxOptions.memoryLimit);
+              "sandbox_" + context.getId(), sandboxOptions.memoryLimitMb);
       commandLineBuilder.setCgroupsDir(cgroupsDir);
     }
 
@@ -410,7 +410,7 @@ protected ImmutableSet<Path> getWritableDirs(
       throws IOException {
     ImmutableSet.Builder<Path> writableDirs = ImmutableSet.builder();
     writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, withinSandboxExecRoot, env));
-    if (getSandboxOptions().memoryLimit > 0) {
+    if (getSandboxOptions().memoryLimitMb > 0) {
       CgroupsInfo cgroupsInfo = CgroupsInfo.getInstance();
       writableDirs.add(fileSystem.getPath(cgroupsInfo.getMountPoint().getAbsolutePath()));
     }

src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java

@@ -20,6 +20,7 @@
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.actions.LocalHostCapacity;
 import com.google.devtools.build.lib.util.OptionsUtils;
+import com.google.devtools.build.lib.util.RamResourceConverter;
 import com.google.devtools.build.lib.util.ResourceConverter;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
@@ -396,14 +397,15 @@ public ImmutableSet<Path> getInaccessiblePaths(FileSystem fs) {
   public boolean sandboxHermeticTmp;
 
   @Option(
-      name = "experimental_sandbox_memory_limit",
+      name = "experimental_sandbox_memory_limit_mb",
       defaultValue = "0",
       documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
       effectTags = {OptionEffectTag.EXECUTION},
+      converter = RamResourceConverter.class,
       help =
-          "If set to true, each Linux sandbox will be limited to the given amount of memory."
-              + " Requires cgroups v2 and permissions for the users to the cgroups dir.")
-  public int memoryLimit;
+          "If > 0, each Linux sandbox will be limited to the given amount of memory (in MB)."
+              + " Requires cgroups v1 or v2 and permissions for the users to the cgroups dir.")
+  public int memoryLimitMb;
 
   /** Converter for the number of threads used for asynchronous tree deletion. */
   public static final class AsyncTreeDeletesConverter extends ResourceConverter {

src/main/java/com/google/devtools/build/lib/util/RamResourceConverter.java

@@ -32,6 +32,6 @@ public RamResourceConverter() {
 
   @Override
   public String getTypeDescription() {
-    return "an integer, or \"HOST_RAM\", optionally followed by [-|*]<float>.";
+    return "an integer number of MBs, or \"HOST_RAM\", optionally followed by [-|*]<float>.";
   }
 }

src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java

@@ -100,7 +100,7 @@ public void buildStarting(BuildStartingEvent event) {
               sandboxOptions.sandboxDebug,
               ImmutableList.copyOf(sandboxOptions.sandboxTmpfsPath),
               ImmutableList.copyOf(sandboxOptions.sandboxWritablePath),
-              sandboxOptions.memoryLimit,
+              sandboxOptions.memoryLimitMb,
               sandboxOptions.getInaccessiblePaths(env.getRuntime().getFileSystem()));
     } else {
       workerSandboxOptions = null;

src/test/shell/integration/sandboxing_test.sh

@@ -269,16 +269,20 @@ function test_sandbox_hardening_with_cgroups_v1() {
 
   mkdir pkg
   cat >pkg/BUILD <<EOF
-genrule(name = "pkg", outs = ["pkg.out"], cmd = "pwd; echo >\$@")
+genrule(
+  name = "pkg",
+  outs = ["pkg.out"],
+  cmd = "pwd; hexdump -C -n 250000 < /dev/random | sort > /dev/null 2>\$@"
+)
 EOF
   local genfiles_base="$(bazel info ${PRODUCT_NAME}-genfiles)"
 
   bazel build --genrule_strategy=linux-sandbox \
-    --experimental_sandbox_memory_limit=1000000 //pkg \
+    --experimental_sandbox_memory_limit_mb=1000 //pkg \
     >"${TEST_log}" 2>&1 || fail "Expected build to succeed"
   rm -f ${genfiles_base}/pkg/pkg.out
   bazel build --genrule_strategy=linux-sandbox \
-    --experimental_sandbox_memory_limit=100 //pkg \
+    --experimental_sandbox_memory_limit_mb=1 //pkg \
     >"${TEST_log}" 2>&1 && fail "Expected build to fail" || true
 }
 
@@ -298,22 +302,26 @@ function test_sandbox_hardening_with_cgroups_v2() {
 
   mkdir pkg
   cat >pkg/BUILD <<EOF
-genrule(name = "pkg", outs = ["pkg.out"], cmd = "pwd; echo >\$@")
+genrule(
+  name = "pkg",
+  outs = ["pkg.out"],
+  cmd = "pwd; hexdump -C -n 250000 < /dev/random | sort > /dev/null 2>\$@"
+)
 EOF
   local genfiles_base="$(bazel info ${PRODUCT_NAME}-genfiles)"
   # Need to make sure the bazel server runs under systemd, too.
   bazel shutdown
 
   XDG_RUNTIME_DIR=/run/user/$( id -u ) systemd-run --user --scope \
   bazel build --genrule_strategy=linux-sandbox \
-    --experimental_sandbox_memory_limit=1000000 //pkg \
+    --experimental_sandbox_memory_limit_mb=1000 //pkg \
     >"${TEST_log}" 2>&1 || fail "Expected build to succeed"
   rm -f ${genfiles_base}/pkg/pkg.out
 
   bazel shutdown
   XDG_RUNTIME_DIR=/run/user/$( id -u ) systemd-run --user --scope \
   bazel build --genrule_strategy=linux-sandbox \
-    --experimental_sandbox_memory_limit=100 //pkg \
+    --experimental_sandbox_memory_limit_mb=1 //pkg \
     >"${TEST_log}" 2>&1 && fail "Expected build to fail" || true
 }