aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGravatar MarkKoz <[email protected]>2021-03-07 23:35:29 -0800
committerGravatar MarkKoz <[email protected]>2021-03-07 23:35:29 -0800
commit46fc7285788c3602e3325dcb2f505dd22e11f513 (patch)
tree09a855b6fefc6470a7314b86dd3f41aa0ccafd0e
parentDocker: improve caching & install numpy in container (diff)
parentMerge pull request #94 from python-discord/recursive-remove-cgroups (diff)
Merge master into bug/tests/nsjail
The branch needs the fixes from #94 to make the tests pass.
-rw-r--r--config/snekbox.cfg2
-rw-r--r--snekbox/nsjail.py40
2 files changed, 34 insertions, 8 deletions
diff --git a/config/snekbox.cfg b/config/snekbox.cfg
index aa39059..257b5ca 100644
--- a/config/snekbox.cfg
+++ b/config/snekbox.cfg
@@ -105,11 +105,9 @@ mount {
cgroup_mem_max: 52428800
cgroup_mem_mount: "/sys/fs/cgroup/memory"
-cgroup_mem_parent: "NSJAIL"
cgroup_pids_max: 1
cgroup_pids_mount: "/sys/fs/cgroup/pids"
-cgroup_pids_parent: "NSJAIL"
iface_no_lo: true
diff --git a/snekbox/nsjail.py b/snekbox/nsjail.py
index d48f72b..c7087ad 100644
--- a/snekbox/nsjail.py
+++ b/snekbox/nsjail.py
@@ -4,6 +4,7 @@ import re
import subprocess
import sys
import textwrap
+import uuid
from pathlib import Path
from subprocess import CompletedProcess
from tempfile import NamedTemporaryFile
@@ -41,8 +42,6 @@ class NsJail:
self.nsjail_binary = nsjail_binary
self.config = self._read_config()
- self._create_parent_cgroups()
-
@staticmethod
def _read_config() -> NsJailConfig:
"""Read the NsJail config at `NSJAIL_CFG` and return a protobuf Message object."""
@@ -66,17 +65,22 @@ class NsJail:
return config
- def _create_parent_cgroups(self) -> None:
+ def _create_dynamic_cgroups(self) -> str:
"""
- Create the PIDs and memory cgroups which NsJail will use as its parent cgroups.
+ Create a PID and memory cgroup for NsJail to use as the parent cgroup.
+
+ Returns the name of the cgroup, located in the cgroup root.
NsJail doesn't do this automatically because it requires privileges NsJail usually doesn't
have.
Disables memory swapping.
"""
- pids = Path(self.config.cgroup_pids_mount, self.config.cgroup_pids_parent)
- mem = Path(self.config.cgroup_mem_mount, self.config.cgroup_mem_parent)
+ # Pick a name for the cgroup
+ cgroup = "snekbox-" + str(uuid.uuid4())
+
+ pids = Path(self.config.cgroup_pids_mount, cgroup)
+ mem = Path(self.config.cgroup_mem_mount, cgroup)
mem_max = str(self.config.cgroup_mem_max)
pids.mkdir(parents=True, exist_ok=True)
@@ -102,6 +106,8 @@ class NsJail:
"Please ensure swap memory is disabled on the system."
)
+ return cgroup
+
@staticmethod
def _parse_log(log_lines: Iterable[str]) -> None:
"""Parse and log NsJail's log messages."""
@@ -171,11 +177,16 @@ class NsJail:
Additional arguments passed will be used to override the values in the NsJail config.
These arguments are only options for NsJail; they do not affect Python's arguments.
"""
+ cgroup = self._create_dynamic_cgroups()
+
with NamedTemporaryFile() as nsj_log:
args = (
self.nsjail_binary,
"--config", NSJAIL_CFG,
"--log", nsj_log.name,
+ # Set our dynamically created parent cgroups
+ "--cgroup_mem_parent", cgroup,
+ "--cgroup_pids_parent", cgroup,
*args,
"--",
self.config.exec_bin.path, *self.config.exec_bin.arg, "-c", code
@@ -211,4 +222,21 @@ class NsJail:
self._parse_log(log_lines)
log.info(f"nsjail return code: {returncode}")
+
+ # If we hit a cgroup limit then there is a chance the nsjail cgroups did not
+ # get removed. If we don't remove them then when we try remove the parents
+ # we will get a "Device or resource busy" error.
+
+ children = []
+
+ children.extend(Path(self.config.cgroup_mem_mount, cgroup).glob("NSJAIL.*"))
+ children.extend(Path(self.config.cgroup_pids_mount, cgroup).glob("NSJAIL.*"))
+
+ for child in children:
+ child.rmdir()
+
+ # Remove the dynamically created cgroups once we're done
+ Path(self.config.cgroup_mem_mount, cgroup).rmdir()
+ Path(self.config.cgroup_pids_mount, cgroup).rmdir()
+
return CompletedProcess(args, returncode, output, None)