diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1699261dbc..eb665e5f98 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -26,6 +26,9 @@ jobs: - name: Run tests run: | + # github runner kernel for some reason doesn't have ext4 quota + # support, but does have tmpfs quotas + export TEST_QUOTAS=tmpfs docker compose -p cms -f docker/docker-compose.test.yml run --rm testcms - name: Upload test results to Codecov diff --git a/cms/conf.py b/cms/conf.py index 6e7495443f..107dea1113 100644 --- a/cms/conf.py +++ b/cms/conf.py @@ -85,14 +85,20 @@ class DatabaseConfig: @dataclass() class WorkerConfig: - keep_sandbox: bool = False + # TODO delete entirely?? + pass + + +@dataclass() +class FSQuotaSettings: + kb: int + inodes: int @dataclass() class SandboxConfig: sandbox_implementation: str = "isolate" - # Max size of each writable file during an evaluation step, in KiB. - max_file_size: int = 1024 * 1024 # 1 GiB + fs_quota: FSQuotaSettings | None = None # Max processes, CPU time (s), memory (KiB) for compilation runs. compilation_sandbox_max_processes: int = 1000 compilation_sandbox_max_time_s: float = 10.0 diff --git a/cms/grading/Job.py b/cms/grading/Job.py index 3b08094d02..5eb19aa328 100644 --- a/cms/grading/Job.py +++ b/cms/grading/Job.py @@ -87,7 +87,6 @@ def __init__( multithreaded_sandbox: bool = False, archive_sandbox: bool = False, shard: int | None = None, - keep_sandbox: bool = False, sandboxes: list[str] | None = None, sandbox_digests: dict[str, str] | None = None, info: str | None = None, @@ -108,9 +107,6 @@ def __init__( allow multithreading. archive_sandbox: whether the sandbox is to be archived. shard: the shard of the Worker completing this job. - keep_sandbox: whether to forcefully keep the sandbox, - even if other conditions (the config, the sandbox status) - don't warrant it. sandboxes: the paths of the sandboxes used in the Worker during the execution of the job. sandbox_digests: the digests of the sandbox archives used to @@ -148,7 +144,6 @@ def __init__( self.multithreaded_sandbox = multithreaded_sandbox self.archive_sandbox = archive_sandbox self.shard = shard - self.keep_sandbox = keep_sandbox self.sandboxes = sandboxes self.sandbox_digests = sandbox_digests self.info = info @@ -172,7 +167,6 @@ def export_to_dict(self) -> dict: 'multithreaded_sandbox': self.multithreaded_sandbox, 'archive_sandbox': self.archive_sandbox, 'shard': self.shard, - 'keep_sandbox': self.keep_sandbox, 'sandboxes': self.sandboxes, 'sandbox_digests': self.sandbox_digests, 'info': self.info, @@ -303,7 +297,6 @@ def __init__( task_type: str | None = None, task_type_parameters: object = None, shard: int | None = None, - keep_sandbox: bool = False, sandboxes: list[str] | None = None, sandbox_digests: dict[str, str] | None = None, info: str | None = None, @@ -330,7 +323,7 @@ def __init__( Job.__init__(self, operation, task_type, task_type_parameters, language, multithreaded_sandbox, archive_sandbox, - shard, keep_sandbox, sandboxes, sandbox_digests, info, success, + shard, sandboxes, sandbox_digests, info, success, text, files, managers, executables) self.compilation_success = compilation_success self.plus = plus @@ -520,7 +513,6 @@ def __init__( task_type: str | None = None, task_type_parameters: object = None, shard: int | None = None, - keep_sandbox: bool = False, sandboxes: list[str] | None = None, sandbox_digests: dict[str, str] | None = None, info: str | None = None, @@ -566,7 +558,7 @@ def __init__( """ Job.__init__(self, operation, task_type, task_type_parameters, language, multithreaded_sandbox, archive_sandbox, - shard, keep_sandbox, sandboxes, sandbox_digests, info, success, + shard, sandboxes, sandbox_digests, info, success, text, files, managers, executables) self.input = input self.output = output diff --git a/cms/grading/Sandbox.py b/cms/grading/Sandbox.py index 7867006902..ceac086ea0 100644 --- a/cms/grading/Sandbox.py +++ b/cms/grading/Sandbox.py @@ -216,7 +216,6 @@ def __init__( # These are not necessarily used, but are here for API compatibility # TODO: move all other common properties here. self.box_id: int = 0 - self.fsize: int | None = None self.dirs: list[tuple[str | None, str, str | None]] = [] self.preserve_env: bool = False self.inherit_env: list[str] = [] @@ -536,29 +535,6 @@ def cleanup(self, delete: bool = False): """ pass - def archive(self) -> str | None: - """Archive the directory where the sandbox operated. - - Stores the archived sandbox in the file cacher and returns its digest. - Returns None if archiving failed. - - """ - logger.info("Archiving sandbox in %s.", self.get_root_path()) - - with tempfile.TemporaryFile(dir=self.temp_dir) as sandbox_archive: - # Archive the working directory - content_path = self.get_root_path() - try: - with tarfile.open(fileobj=sandbox_archive, mode='w:gz') as tar_file: - tar_file.add(content_path, os.path.basename(content_path)) - except Exception: - logger.warning("Failed to archive sandbox", exc_info=True) - return None - - # Put archive to FS - sandbox_archive.seek(0) - return self.file_cacher.put_file_from_fobj(sandbox_archive, "Sandbox %s" % self.get_root_path()) - class StupidSandbox(SandboxBase): """A stupid sandbox implementation. It has very few features and @@ -891,11 +867,6 @@ class IsolateSandbox(SandboxBase): """ next_id = 0 - # If the command line starts with this command name, we are just - # going to execute it without sandboxing, and with all permissions - # on the current directory. - SECURE_COMMANDS = ["/bin/cp", "/bin/mv", "/usr/bin/zip", "/usr/bin/unzip"] - def __init__(self, file_cacher, name=None, temp_dir=None): """Initialization. @@ -918,22 +889,25 @@ def __init__(self, file_cacher, name=None, temp_dir=None): box_id = IsolateSandbox.next_id % 10 IsolateSandbox.next_id += 1 - # We create a directory "home" inside the outer temporary directory, - # that will be bind-mounted to "/tmp" inside the sandbox (some - # compilers need "/tmp" to exist, and this is a cheap shortcut to - # create it). The sandbox also runs code as a different user, and so - # we need to ensure that they can read and write to the directory. - # But we don't want everybody on the system to, which is why the - # outer directory exists with no read permissions. - self._outer_dir = tempfile.mkdtemp(dir=self.temp_dir, - prefix="cms-%s-" % (self.name)) - self._home = os.path.join(self._outer_dir, "home") - self._home_dest = "/tmp" - os.mkdir(self._home) - self.allow_writing_all() - self.exec_name = 'isolate' self.box_exec = self.detect_box_executable() + self.box_id = box_id + + # Tell isolate to get the sandbox ready. We do our best to cleanup + # after ourselves, but we might have missed something if a previous + # worker was interrupted in the middle of an execution, so we issue an + # idempotent cleanup. + self.cleanup() + + isolate_box_dir = self.initialize_isolate() + + # self._outer_dir = isolate_box_dir + self._outer_dir = tempfile.mkdtemp( + dir=self.temp_dir, prefix=f"cms-{self.name}-" + ) + self._home = os.path.join(isolate_box_dir, "box") + self._home_dest = "/box" + # Used for -M - the meta file ends up in the outer directory. The # actual filename will be .. self.info_basename = os.path.join(self._outer_dir, "run.log") @@ -946,13 +920,11 @@ def __init__(self, file_cacher, name=None, temp_dir=None): self._home, self.box_exec) # Default parameters for isolate - self.box_id = box_id # -b self.chdir = self._home_dest # -c self.dirs = [] # -d self.preserve_env = False # -e self.inherit_env = [] # -E self.set_env = {} # -E - self.fsize = None # -f self.stdin_file: str | None = None # -i self.stack_space: int | None = None # -k self.address_space: int | None = None # -m @@ -963,9 +935,6 @@ def __init__(self, file_cacher, name=None, temp_dir=None): self.wallclock_timeout: float | None = None # -w self.extra_timeout: float | None = None # -x - self.add_mapped_directory( - self._home, dest=self._home_dest, options="rw") - # Create temporary directory on /dev/shm to prevent communication # between sandboxes. self.dirs.append((None, "/dev/shm", "tmp")) @@ -984,13 +953,6 @@ def __init__(self, file_cacher, name=None, temp_dir=None): # particular, the System.Native assembly. self.maybe_add_mapped_directory("/etc/mono", options="noexec") - # Tell isolate to get the sandbox ready. We do our best to cleanup - # after ourselves, but we might have missed something if a previous - # worker was interrupted in the middle of an execution, so we issue an - # idempotent cleanup. - self.cleanup() - self.initialize_isolate() - def add_mapped_directory( self, src: str, @@ -1021,61 +983,6 @@ def maybe_add_mapped_directory( return self.add_mapped_directory(src, dest, options, ignore_if_not_existing=True) - def allow_writing_all(self): - """Set permissions in such a way that any operation is allowed. - - """ - os.chmod(self._home, 0o777) - for filename in os.listdir(self._home): - os.chmod(os.path.join(self._home, filename), 0o777) - - def allow_writing_none(self): - """Set permissions in such a way that the user cannot write anything. - - """ - os.chmod(self._home, 0o755) - for filename in os.listdir(self._home): - os.chmod(os.path.join(self._home, filename), 0o755) - - def allow_writing_only(self, inner_paths: list[str]): - """Set permissions in so that the user can write only some paths. - - By default the user can only write to the home directory. This - method further restricts permissions so that it can only write - to some files inside the home directory. - - inner_paths: the only paths that the user is allowed to - write to; they should be "inner" paths (from the perspective - of the sandboxed process, not of the host system); they can - be absolute or relative (in which case they are interpreted - relative to the home directory); paths that point to a file - outside the home directory are ignored. - - """ - outer_paths = [] - for inner_path in inner_paths: - abs_inner_path = \ - os.path.realpath(os.path.join(self._home_dest, inner_path)) - # If an inner path is absolute (e.g., /fifo0/u0_to_m) then - # it may be outside home and we should ignore it. - if os.path.commonpath([abs_inner_path, - self._home_dest]) != self._home_dest: - continue - rel_inner_path = os.path.relpath(abs_inner_path, self._home_dest) - outer_path = os.path.join(self._home, rel_inner_path) - outer_paths.append(outer_path) - - # If one of the specified file do not exists, we touch it to - # assign the correct permissions. - for path in outer_paths: - if not os.path.exists(path): - open(path, "wb").close() - - # Close everything, then open only the specified. - self.allow_writing_none() - for path in outer_paths: - os.chmod(path, 0o722) - def get_root_path(self) -> str: """Return the toplevel path of the sandbox. @@ -1103,8 +1010,11 @@ def detect_box_executable(self) -> str: return: the path to a valid (hopefully) isolate. """ + # TODO do we need this logic?? + # i don't think isolate is usable without installing anyways, you need the config file. paths = [os.path.join('.', 'isolate', self.exec_name), os.path.join('.', self.exec_name)] + # in any case this location isn't useful, since we don't have isolate as a submodule anymore. if '__file__' in globals(): paths += [os.path.abspath(os.path.join( os.path.dirname(__file__), @@ -1148,9 +1058,6 @@ def build_box_options(self) -> list[str]: res += ["--env=%s" % var] for var, value in self.set_env.items(): res += ["--env=%s=%s" % (var, value)] - if self.fsize is not None: - # Isolate wants file size as KiB. - res += ["--fsize=%d" % (self.fsize // 1024)] if self.stdin_file is not None: res += ["--stdin=%s" % self.inner_absolute_path(self.stdin_file)] if self.stack_space is not None: @@ -1358,44 +1265,13 @@ def _popen( self.log = None self.exec_num += 1 - # We run a selection of commands without isolate, as they need - # to create new files. This is safe because these commands do - # not depend on the user input. - if command[0] in IsolateSandbox.SECURE_COMMANDS: - logger.debug("Executing non-securely: %s at %s", - pretty_print_cmdline(command), self._home) - try: - prev_permissions = stat.S_IMODE(os.stat(self._home).st_mode) - os.chmod(self._home, 0o700) - with open(self.cmd_file, 'at', encoding="utf-8") as cmds: - cmds.write("%s\n" % (pretty_print_cmdline(command))) - p = subprocess.Popen(command, cwd=self._home, - stdin=stdin, stdout=stdout, stderr=stderr, - close_fds=close_fds) - os.chmod(self._home, prev_permissions) - # For secure commands, we clear the output so that it - # is not forwarded to the contestants. Secure commands - # are "setup" commands, which should not fail or - # provide information for the contestants. - open(os.path.join(self._home, self.stdout_file), "wb").close() - open(os.path.join(self._home, self.stderr_file), "wb").close() - self._write_empty_run_log(self.exec_num) - except OSError: - logger.critical( - "Failed to execute program in sandbox with command: %s", - pretty_print_cmdline(command), exc_info=True) - raise - return p - args = [self.box_exec] + self.build_box_options() + ["--"] + command logger.debug("Executing program in sandbox with command: `%s'.", pretty_print_cmdline(args)) - # Temporarily allow writing new files. - prev_permissions = stat.S_IMODE(os.stat(self._home).st_mode) - os.chmod(self._home, 0o770) - with open(self.cmd_file, 'at', encoding="utf-8") as commands: - commands.write("%s\n" % (pretty_print_cmdline(args))) - os.chmod(self._home, prev_permissions) + + with open(self.cmd_file, "a") as commands: + commands.write(pretty_print_cmdline(args) + "\n") + try: p = subprocess.Popen(args, stdin=stdin, stdout=stdout, stderr=stderr, @@ -1408,15 +1284,6 @@ def _popen( return p - def _write_empty_run_log(self, index: int): - """Write a fake run.log file with no information.""" - info_file = "%s.%d" % (self.info_basename, index) - with open(info_file, "wt", encoding="utf-8") as f: - f.write("time:0.000\n") - f.write("time-wall:0.000\n") - f.write("max-rss:0\n") - f.write("cg-mem:0\n") - def execute_without_std( self, command: list[str], wait: bool = False ) -> bool | subprocess.Popen: @@ -1469,35 +1336,25 @@ def translate_box_exitcode(self, exitcode: int) -> bool: raise SandboxInterfaceException("Sandbox exit status (%d) unknown" % exitcode) - def initialize_isolate(self): + def initialize_isolate(self) -> str: """Initialize isolate's box.""" init_cmd = [self.box_exec, "--box-id=%d" % self.box_id, "--cg", "--init"] + + quota_cfg = config.sandbox.fs_quota + if quota_cfg is not None: + init_cmd += [f"--quota={quota_cfg.kb},{quota_cfg.inodes}"] + try: - subprocess.check_call(init_cmd) + return subprocess.run(init_cmd, check=True, + capture_output=True, encoding="utf-8").stdout.strip() except subprocess.CalledProcessError as e: raise SandboxInterfaceException( "Failed to initialize sandbox") from e - def cleanup(self, delete=False): + def cleanup(self, delete: bool = False): """See Sandbox.cleanup().""" - # The user isolate assigns within the sandbox might have created - # subdirectories and files therein, making the user outside the sandbox - # unable to delete the whole tree. If the caller asked us to delete the - # sandbox, we first issue a chmod within isolate to make sure that we - # will be able to delete everything. If not, we leave the files as they - # are to avoid masking possible problems the admin wanted to debug. - exe = [self.box_exec, "--box-id=%d" % self.box_id, "--cg"] - if delete: - # Ignore exit status as some files may be owned by our user - subprocess.call( - exe + [ - "--dir=%s=%s:rw" % (self._home_dest, self._home), - "--run", "--", - "/bin/chmod", "777", "-R", self._home_dest], - stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT) - # Tell isolate to cleanup the sandbox. subprocess.check_call( exe + ["--cleanup"], @@ -1508,6 +1365,37 @@ def cleanup(self, delete=False): # Delete the working directory. rmtree(self._outer_dir) + def archive(self) -> str | None: + """Archive the directory where the sandbox operated. + + Stores the archived sandbox in the file cacher and returns its digest. + Returns None if archiving failed. + + """ + logger.info("Archiving sandbox in %s.", self.get_root_path()) + + with tempfile.TemporaryFile(dir=self.temp_dir) as sandbox_archive: + # Archive the working directory + metadata_path = self.get_root_path() + box_dir = self._home + with tarfile.open(fileobj=sandbox_archive, mode='w:gz') as tar_file: + try: + # Add metadata files + for x in os.listdir(metadata_path): + tar_file.add(os.path.join(metadata_path, x), x) + # Add the box directory + tar_file.add(box_dir, "box") + except Exception: + logger.warning( + "Failed to add files to sandbox archive", exc_info=True + ) + + # Put archive to FS + sandbox_archive.seek(0) + return self.file_cacher.put_file_from_fobj( + sandbox_archive, f"Sandbox {self.get_root_path()}" + ) + # StupidSandbox hasn't been tested in a while. It should probably be removed, # but for now, for typing purposes let's assume we're always using diff --git a/cms/grading/steps/evaluation.py b/cms/grading/steps/evaluation.py index adbe507b32..f62ac38ed0 100644 --- a/cms/grading/steps/evaluation.py +++ b/cms/grading/steps/evaluation.py @@ -86,7 +86,6 @@ def evaluation_step( time_limit: float | None = None, memory_limit: int | None = None, dirs_map: dict[str, tuple[str | None, str | None]] | None = None, - writable_files: list[str] | None = None, stdin_redirect: str | None = None, stdout_redirect: str | None = None, multiprocess: bool = False, @@ -109,11 +108,6 @@ def evaluation_step( from external directories to a pair of strings: the first is the path they should be mapped to inside the sandbox, the second, is isolate's options for the mapping. - writable_files: a list of inner file names (relative to - the inner path) on which the command is allow to write, or None to - indicate that all files are read-only; if applicable, redirected - output and the standard error are implicitly added to the files - allowed. stdin_redirect: the name of the file that will be redirected to the standard input of each command; if None, nothing will be provided to stdin. @@ -136,7 +130,7 @@ def evaluation_step( for command in commands: success = evaluation_step_before_run( sandbox, command, time_limit, memory_limit, - dirs_map, writable_files, stdin_redirect, stdout_redirect, + dirs_map, stdin_redirect, stdout_redirect, multiprocess, wait=True) if not success: logger.debug("Job failed in evaluation_step_before_run.") @@ -155,7 +149,6 @@ def evaluation_step_before_run( time_limit: float | None = None, memory_limit: int | None = None, dirs_map: dict[str, tuple[str | None, str | None]] | None = None, - writable_files: list[str] | None = None, stdin_redirect: str | None = None, stdout_redirect: str | None = None, multiprocess: bool = False, @@ -182,8 +175,6 @@ def evaluation_step_before_run( # Default parameters handling. if dirs_map is None: dirs_map = {} - if writable_files is None: - writable_files = [] if stdout_redirect is None: stdout_redirect = "stdout.txt" @@ -200,19 +191,12 @@ def evaluation_step_before_run( else: sandbox.address_space = None - # config.sandbox.max_file_size is in KiB - sandbox.fsize = config.sandbox.max_file_size * 1024 - sandbox.stdin_file = stdin_redirect sandbox.stdout_file = stdout_redirect sandbox.stderr_file = "stderr.txt" for src, (dest, options) in dirs_map.items(): sandbox.add_mapped_directory(src, dest=dest, options=options) - for name in [sandbox.stderr_file, sandbox.stdout_file]: - if name is not None: - writable_files.append(name) - sandbox.allow_writing_only(writable_files) sandbox.set_multiprocess(multiprocess) diff --git a/cms/grading/tasktypes/Batch.py b/cms/grading/tasktypes/Batch.py index d23ab04e51..93c0168ea4 100644 --- a/cms/grading/tasktypes/Batch.py +++ b/cms/grading/tasktypes/Batch.py @@ -281,18 +281,13 @@ def _execution_step(self, job, file_cacher): self._actual_input: job.input } - # Check which redirect we need to perform, and in case we don't - # manage the output via redirect, the submission needs to be able - # to write on it. - files_allowing_write = [] + # Check which redirect we need to perform stdin_redirect = None stdout_redirect = None if len(self.input_filename) == 0: stdin_redirect = self._actual_input if len(self.output_filename) == 0: stdout_redirect = self._actual_output - else: - files_allowing_write.append(self._actual_output) # Create the sandbox sandbox = create_sandbox(file_cacher, name="evaluate") @@ -310,7 +305,6 @@ def _execution_step(self, job, file_cacher): commands, job.time_limit, job.memory_limit, - writable_files=files_allowing_write, stdin_redirect=stdin_redirect, stdout_redirect=stdout_redirect, multiprocess=job.multithreaded_sandbox) diff --git a/cms/grading/tasktypes/Communication.py b/cms/grading/tasktypes/Communication.py index 60f53a993b..8bf1244f31 100644 --- a/cms/grading/tasktypes/Communication.py +++ b/cms/grading/tasktypes/Communication.py @@ -323,7 +323,6 @@ def evaluate(self, job, file_cacher): manager_time_limit, config.sandbox.trusted_sandbox_max_memory_kib * 1024, dirs_map=dict((fifo_dir[i], (sandbox_fifo_dir[i], "rw")) for i in indices), - writable_files=[self.OUTPUT_FILENAME], stdin_redirect=self.INPUT_FILENAME, multiprocess=job.multithreaded_sandbox, ) @@ -437,6 +436,6 @@ def evaluate(self, job, file_cacher): delete_sandbox(sandbox_mgr, job) for s in sandbox_user: delete_sandbox(s, job) - if job.success and not config.worker.keep_sandbox and not job.keep_sandbox: + if job.success: for d in fifo_dir: rmtree(d) diff --git a/cms/grading/tasktypes/util.py b/cms/grading/tasktypes/util.py index 609d7c5ac6..7d9e87aabf 100644 --- a/cms/grading/tasktypes/util.py +++ b/cms/grading/tasktypes/util.py @@ -82,19 +82,16 @@ def delete_sandbox(sandbox: Sandbox, job: Job, success: bool | None = None): success = job.success # Archive the sandbox if required - if job.archive_sandbox: + if job.archive_sandbox or not success: sandbox_digest = sandbox.archive() if sandbox_digest is not None: job.sandbox_digests[sandbox.get_root_path()] = sandbox_digest - # If the job was not successful, we keep the sandbox around. - if not success: - logger.warning("Sandbox %s kept around because job did not succeed.", - sandbox.get_root_path()) + if not success: + logger.warning(f"Job did not succeed! Sandbox digest: {sandbox_digest}") - delete = success and not config.worker.keep_sandbox and not job.keep_sandbox try: - sandbox.cleanup(delete=delete) + sandbox.cleanup(delete=True) except OSError: err_msg = "Couldn't delete sandbox." logger.warning(err_msg, exc_info=True) diff --git a/cmstestsuite/Tests.py b/cmstestsuite/Tests.py index 03870f6d49..176829d9dc 100644 --- a/cmstestsuite/Tests.py +++ b/cmstestsuite/Tests.py @@ -20,6 +20,8 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import os + import cmstestsuite.tasks.batch_and_output as batch_and_output import cmstestsuite.tasks.batch_fileio as batch_fileio import cmstestsuite.tasks.batch_fileio_managed as batch_fileio_managed @@ -436,34 +438,6 @@ checks=[CheckCompilationFail()]), - # Writing to files not allowed. - - # Inability to write to a file does not throw a specific error, - # just returns a NULL file handler to the caller. So we rely on - # the test program to write the correct result only if the - # returned handler is valid. - - Test('write-forbidden-fileio', - task=batch_fileio, filenames=['write-forbidden-fileio.%l'], - languages=(LANG_C,), - checks=[CheckOverallScore(0, 100)]), - - Test('write-forbidden-stdio', - task=batch_stdio, filenames=['write-forbidden-stdio.%l'], - languages=(LANG_C,), - checks=[CheckOverallScore(0, 100)]), - - Test('write-forbidden-managed', - task=batch_fileio_managed, filenames=['write-forbidden-managed.%l'], - languages=(LANG_C,), - checks=[CheckOverallScore(0, 100)]), - - Test('write-forbidden-communication', - task=communication_fifoio_stubbed, - filenames=['write-forbidden-communication.%l'], - languages=(LANG_C,), - checks=[CheckOverallScore(0, 100)]), - # This tests complete successfully only if it is unable to execute # output.txt. @@ -484,11 +458,17 @@ languages=(LANG_C,), checks=[CheckOverallScore(0, 100)]), - # Write a huge file - - Test('write-big-fileio', - task=batch_fileio, filenames=['write-big-fileio.%l'], - languages=(LANG_C,), - checks=[CheckOverallScore(0, 100)]), - ] + +# TODO figure out a better way to enable/disable this......... +if os.environ.get('TEST_QUOTAS', '') != '': + ALL_TESTS += [ + Test('write-many-files', + task=batch_fileio, filenames=['write-many-files.%l'], + languages=(LANG_C,), + checks=[CheckOverallScore(100, 100)]), + Test('write-big-file-quota', + task=batch_fileio, filenames=['write-big-file-quota.%l'], + languages=(LANG_C,), + checks=[CheckOverallScore(100, 100)]) + ] diff --git a/cmstestsuite/code/write-big-file-quota.c b/cmstestsuite/code/write-big-file-quota.c new file mode 100644 index 0000000000..90af9d752f --- /dev/null +++ b/cmstestsuite/code/write-big-file-quota.c @@ -0,0 +1,31 @@ + +#include +#include +#include +#include +#include + +int main() { + int n, i; + FILE* in = fopen("input.txt", "r"); + fscanf(in, "%d", &n); + + FILE* temp = fopen("temp.txt", "w"); + // The quota is set to 64MB, so this should fail. + int size = 65*1024*1024; + char* buf = calloc(size, 1); + size_t res = fwrite(buf, 1, size, temp); + int write_error = errno; + fclose(temp); + // Clean up the temporary file, so that we have space for output.txt + unlink("temp.txt"); + FILE* out = fopen("output.txt", "w"); + + if (res < size && write_error == EDQUOT) { + fprintf(out, "correct %d\n", n); + return 0; + } else { + fprintf(out, "incorrect %d\n%d", n, write_error); + return 0; + } +} diff --git a/cmstestsuite/code/write-big-fileio.c b/cmstestsuite/code/write-big-fileio.c deleted file mode 100644 index e1d959d59f..0000000000 --- a/cmstestsuite/code/write-big-fileio.c +++ /dev/null @@ -1,35 +0,0 @@ -#include - -#include -#include -#include -#include - -/* First we create a huge file in output.txt (2 GB). If the check on - file size is enforced, the write fails and we write a wrong - output. Otherwise we write correct output. */ - -int main() { - int n, i; - FILE *in = fopen("input.txt", "r"); - fscanf(in, "%d", &n); - - // Seek is done in two steps so there are no probles even when - // type off_t is 4 bytes long. In this part everything is done - // calling directly syscalls because apparently libc does not - // properly report EFBIG errors. - int outfd = open("output.txt", O_WRONLY|O_CREAT|O_TRUNC, 0666); - lseek(outfd, 1024 * 1024 * 1024, SEEK_CUR); - lseek(outfd, 1024 * 1024 * 1024, SEEK_CUR); - i = write(outfd, "\0", 1); - close(outfd); - - FILE *out = fopen("output.txt", "w"); - if (i != 1) { - fprintf(out, "incorrect %d\n", n); - return 1; - } else { - fprintf(out, "correct %d\n", n); - return 0; - } -} diff --git a/cmstestsuite/code/write-forbidden-communication.c b/cmstestsuite/code/write-forbidden-communication.c deleted file mode 100644 index 1206752a3b..0000000000 --- a/cmstestsuite/code/write-forbidden-communication.c +++ /dev/null @@ -1,10 +0,0 @@ -#include - -int userfunc(int x) { - FILE *out = fopen("output.txt", "w"); - FILE *other = fopen("other.txt", "w"); - if (out != NULL ||other != NULL) { - return x; - } - return -1; -} diff --git a/cmstestsuite/code/write-forbidden-fileio.c b/cmstestsuite/code/write-forbidden-fileio.c deleted file mode 100644 index 3abda07084..0000000000 --- a/cmstestsuite/code/write-forbidden-fileio.c +++ /dev/null @@ -1,13 +0,0 @@ -#include - -int main() { - int n; - FILE *in = fopen("input.txt", "r"); - FILE *out = fopen("output.txt", "w"); - FILE *other = fopen("other.txt", "w"); - if (other != NULL) { - fscanf(in, "%d", &n); - fprintf(out, "correct %d\n", n); - } - return 0; -} diff --git a/cmstestsuite/code/write-forbidden-managed.c b/cmstestsuite/code/write-forbidden-managed.c deleted file mode 100644 index 777c534ebf..0000000000 --- a/cmstestsuite/code/write-forbidden-managed.c +++ /dev/null @@ -1,12 +0,0 @@ -#include - -int userfunc(int x) { - // The submission is allowed to access output.txt, that is the - // file where the output of the grader will be written. The grader - // should be designed in such a way that this is not an advantage. - FILE *other = fopen("other.txt", "w"); - if (other != NULL) { - return x; - } - return -1; -} diff --git a/cmstestsuite/code/write-forbidden-stdio.c b/cmstestsuite/code/write-forbidden-stdio.c deleted file mode 100644 index 7ceefdf30d..0000000000 --- a/cmstestsuite/code/write-forbidden-stdio.c +++ /dev/null @@ -1,14 +0,0 @@ -#include - -int main() { - int n; - // The submission is allowed (for technical reasons) to access - // output.txt, that is the file where the standard output will be - // redirected. This is no advantage though. - FILE *other = fopen("other.txt", "w"); - if (other != NULL) { - scanf("%d", &n); - printf("correct %d\n", n); - } - return 0; -} diff --git a/cmstestsuite/code/write-many-files.c b/cmstestsuite/code/write-many-files.c new file mode 100644 index 0000000000..13e8c143c3 --- /dev/null +++ b/cmstestsuite/code/write-many-files.c @@ -0,0 +1,29 @@ +#include +#include + +int main() { + int n, i; + FILE* in = fopen("input.txt", "r"); + fscanf(in, "%d", &n); + fclose(in); + + FILE* out = fopen("output.txt", "w"); + + for(i = 0; i < 1025; i++) { + char outname[32]; + sprintf(outname, "out_%d.txt", i); + FILE* f = fopen(outname, "w"); + if(!f && errno == EDQUOT) { + break; + } + if(f) fclose(f); + } + + if (i >= 1000 && i < 1025) { + fprintf(out, "correct %d\n", n); + return 0; + } else { + fprintf(out, "incorrect %d\n", n); + return 0; + } +} diff --git a/cmstestsuite/unit_tests/grading/steps/fakeisolatesandbox.py b/cmstestsuite/unit_tests/grading/steps/fakeisolatesandbox.py index 1687f0f096..97b60bb935 100644 --- a/cmstestsuite/unit_tests/grading/steps/fakeisolatesandbox.py +++ b/cmstestsuite/unit_tests/grading/steps/fakeisolatesandbox.py @@ -22,8 +22,10 @@ import os from collections import deque from io import BytesIO, StringIO +import tempfile from cms.grading.Sandbox import IsolateSandbox +from cms.util import rmtree class FakeIsolateSandbox(IsolateSandbox): @@ -139,7 +141,10 @@ def execute_without_std(self, command, wait=False): return data["success"] def initialize_isolate(self): - pass + tmpd = tempfile.mkdtemp(dir=self.temp_dir, prefix="cms-fake-sandbox-") + os.mkdir(tmpd+"/box") + return tmpd def cleanup(self): - pass + if hasattr(self, '_outer_dir'): + rmtree(self._outer_dir) diff --git a/cmstestsuite/unit_tests/grading/tasktypes/BatchTest.py b/cmstestsuite/unit_tests/grading/tasktypes/BatchTest.py index f56935111a..e7f1fdaf88 100755 --- a/cmstestsuite/unit_tests/grading/tasktypes/BatchTest.py +++ b/cmstestsuite/unit_tests/grading/tasktypes/BatchTest.py @@ -225,7 +225,7 @@ def test_alone_sandbox_failure(self): self.assertResultsInJob(job) sandbox.get_file_to_storage.assert_not_called() # We preserve the sandbox to let admins check the problem. - sandbox.cleanup.assert_called_once_with(delete=False) + sandbox.archive.assert_called_once() def test_grader_success(self): # We sprinkle in also a header, that should be copied, but not the @@ -355,7 +355,6 @@ def test_stdio_diff_success(self): sandbox, fake_evaluation_commands(EVALUATION_COMMAND_1, "foo", "foo"), 2.5, 123 * 1024 * 1024, - writable_files=[], stdin_redirect="input.txt", stdout_redirect="output.txt", multiprocess=True) @@ -412,7 +411,7 @@ def test_stdio_diff_evaluation_step_sandbox_failure_(self): self.assertResultsInJob(job) # eval_output should not have been called, and the sandbox not deleted. self.eval_output.assert_not_called() - sandbox.cleanup.assert_called_once_with(delete=False) + sandbox.archive.assert_called_once() def test_stdio_diff_eval_output_failure_(self): tt, job = self.prepare(["alone", ["", ""], "diff"], {"foo": EXE_FOO}) @@ -424,7 +423,7 @@ def test_stdio_diff_eval_output_failure_(self): self.assertResultsInJob(job) # Even if the error is in the eval_output sandbox, we keep also the one # for evaluation_step to allow debugging. - sandbox.cleanup.assert_called_once_with(delete=False) + sandbox.archive.assert_called_once() def test_stdio_diff_get_output_success(self): tt, job = self.prepare(["alone", ["", ""], "diff"], {"foo": EXE_FOO}) @@ -476,7 +475,6 @@ def test_fileio_diff_success(self): sandbox, fake_evaluation_commands(EVALUATION_COMMAND_1, "foo", "foo"), 2.5, 123 * 1024 * 1024, - writable_files=["myout"], stdin_redirect=None, stdout_redirect=None, multiprocess=True) diff --git a/cmstestsuite/unit_tests/grading/tasktypes/CommunicationTest.py b/cmstestsuite/unit_tests/grading/tasktypes/CommunicationTest.py index bdd298dd0c..80c5d715aa 100755 --- a/cmstestsuite/unit_tests/grading/tasktypes/CommunicationTest.py +++ b/cmstestsuite/unit_tests/grading/tasktypes/CommunicationTest.py @@ -203,7 +203,7 @@ def test_one_file_sandbox_failure(self): self.assertResultsInJob(job, False, None, None, None) sandbox.get_file_to_storage.assert_not_called() # We preserve the sandbox to let admins check the problem. - sandbox.cleanup.assert_called_once_with(delete=False) + sandbox.archive.assert_called_once() def test_many_files_success(self): tt, job = self.prepare( @@ -390,7 +390,6 @@ def test_single_process_success(self): self.evaluation_step_before_run.assert_has_calls([ call(sandbox_mgr, cmdline_mgr, 4321, 1234 * 1024 * 1024, dirs_map={os.path.join(self.base_dir, "0"): ("/fifo0", "rw")}, - writable_files=["output.txt"], stdin_redirect="input.txt", multiprocess=True), call(sandbox_usr, cmdline_usr, 2.5, 123 * 1024 * 1024, dirs_map={os.path.join(self.base_dir, "0"): ("/fifo0", "rw")}, @@ -419,7 +418,7 @@ def test_single_process_success_long_time_limit(self): self.evaluation_step_before_run.assert_has_calls([ call(sandbox_mgr, ANY, 2.5 + 1, ANY, dirs_map=ANY, - writable_files=ANY, stdin_redirect=ANY, multiprocess=ANY)]) + stdin_redirect=ANY, multiprocess=ANY)]) def test_single_process_missing_manager(self): # Manager is missing, should terminate without creating sandboxes. @@ -466,8 +465,8 @@ def test_single_process_manager_failure(self): tt.evaluate(job, self.file_cacher) self.assertResultsInJob(job, False, None, None, None) - sandbox_mgr.cleanup.assert_called_once_with(delete=False) - sandbox_usr.cleanup.assert_called_once_with(delete=False) + sandbox_mgr.archive.assert_called_once() + sandbox_usr.archive.assert_called_once() def test_single_process_manager_sandbox_failure(self): # Manager sandbox had problems, it's not the user's fault. @@ -484,8 +483,8 @@ def test_single_process_manager_sandbox_failure(self): tt.evaluate(job, self.file_cacher) self.assertResultsInJob(job, False, None, None, None) - sandbox_mgr.cleanup.assert_called_once_with(delete=False) - sandbox_usr.cleanup.assert_called_once_with(delete=False) + sandbox_mgr.archive.assert_called_once() + sandbox_usr.archive.assert_called_once() def test_single_process_manager_and_user_failure(self): # Manager had problems, it's not the user's fault even if also their @@ -503,8 +502,8 @@ def test_single_process_manager_and_user_failure(self): tt.evaluate(job, self.file_cacher) self.assertResultsInJob(job, False, None, None, None) - sandbox_mgr.cleanup.assert_called_once_with(delete=False) - sandbox_usr.cleanup.assert_called_once_with(delete=False) + sandbox_mgr.archive.assert_called_once() + sandbox_usr.archive.assert_called_once() def test_single_process_user_sandbox_failure(self): # User sandbox had problems, it's not the user's fault. @@ -521,8 +520,8 @@ def test_single_process_user_sandbox_failure(self): tt.evaluate(job, self.file_cacher) self.assertResultsInJob(job, False, None, None, None) - sandbox_mgr.cleanup.assert_called_once_with(delete=False) - sandbox_usr.cleanup.assert_called_once_with(delete=False) + sandbox_mgr.archive.assert_called_once() + sandbox_usr.archive.assert_called_once() def test_single_process_user_failure(self): # User program had problems, it's the user's fault. @@ -644,7 +643,6 @@ def test_many_processes_success(self): os.path.join(self.base_dir, "0"): ("/fifo0", "rw"), os.path.join(self.base_dir, "1"): ("/fifo1", "rw"), }, - writable_files=["output.txt"], stdin_redirect="input.txt", multiprocess=True), call(sandbox_usr0, cmdline_usr0, 2.5, 123 * 1024 * 1024, dirs_map={os.path.join(self.base_dir, "0"): ("/fifo0", "rw")}, @@ -681,7 +679,7 @@ def test_many_processes_success_long_time_limit(self): self.evaluation_step_before_run.assert_has_calls([ call(sandbox_mgr, ANY, 2 * (2.5 + 1), ANY, dirs_map=ANY, - writable_files=ANY, stdin_redirect=ANY, multiprocess=ANY)]) + stdin_redirect=ANY, multiprocess=ANY)]) def test_many_processes_first_user_failure(self): # One of the user programs had problems, it's the user's fault. diff --git a/cmstestsuite/unit_tests/grading/tasktypes/tasktypetestutils.py b/cmstestsuite/unit_tests/grading/tasktypes/tasktypetestutils.py index 5d3982b5c3..0a55de0f25 100644 --- a/cmstestsuite/unit_tests/grading/tasktypes/tasktypetestutils.py +++ b/cmstestsuite/unit_tests/grading/tasktypes/tasktypetestutils.py @@ -95,11 +95,6 @@ def setUpMocks(self, tasktype: str): """ self.tasktype = tasktype - # Ensure we don't retain all sandboxes so we can verify delete(). - patcher = patch.object(config.worker, "keep_sandbox", False) - self.addCleanup(patcher.stop) - patcher.start() - # Mock the set of languages (if the task type uses it). Child classes # can update this dict before the test to change the set of languages # CMS understands. diff --git a/config/cms.sample.toml b/config/cms.sample.toml index f609ffe089..76a7d47f54 100644 --- a/config/cms.sample.toml +++ b/config/cms.sample.toml @@ -77,19 +77,33 @@ debug = false twophase_commit = false -[worker] -# Don't delete the sandbox directory under /tmp/ when they are not -# needed anymore. Warning: this can easily eat GB of space very soon. -keep_sandbox = false - - [sandbox] # Which sandbox implementation to use. Currently only isolate is # supported. sandbox_implementation = "isolate" -# Do not allow contestants' solutions to write files bigger than this -# size (expressed in KB; defaults to 1 GB). -max_file_size = 1_048_576 + +# If these are set, enforce a filesystem quota on sandboxes. Note that: +# (1) The file system that stores isolate boxes (box_root in isolate's +# config file) must have quota accounting enabled (for a tmpfs, +# mounting with the usrquota mount option is sufficient; for ext4, +# run `tune2fs -O quota /dev/sdXY` while unmounted, then mount with +# the usrquota option). +# (2) If you cannot configure disk quotas for some reason (e.g. when +# running a kernel without quota support), you can instead put +# isolate's box_root on a tmpfs; this way, all written files count +# towards the solution's memory usage. In that case, do not set +# these two options. +# (3) This quota is used for all types of sandboxes (including +# compilation and checker runs) and includes all files in the +# sandbox (including inputs, outputs, and the submission executable, +# and files written to /tmp). +# (4) You must set both the size and inode limit. + +# This is the maximum size (in kibibytes) of the sandbox's home +# directory (as reported by e.g. `du`). +#fs_quota.kb = 65536 +# Maximum number of inodes (i.e. files) in the sandbox's home directory. +#fs_quota.inodes = 1024 # Max processes, CPU time (s), memory (KiB) for compilation runs. compilation_sandbox_max_processes = 1000 diff --git a/docker/_cms-test-internal.sh b/docker/_cms-test-internal.sh index 6608e5ef47..ae1acf1976 100755 --- a/docker/_cms-test-internal.sh +++ b/docker/_cms-test-internal.sh @@ -13,6 +13,29 @@ dropdb --host=testdb --username=postgres cmsdbfortesting createdb --host=testdb --username=postgres cmsdbfortesting cmsInitDB +if [ -n $TEST_QUOTAS ]; then + sed -i 's/#fs_quota/fs_quota/' /home/cmsuser/cms/etc/cms-testdb.toml + # Some platforms only support quotas on ext4, some only support them + # on tmpfs. So pick the filesystem accordingly. + case $TEST_QUOTAS in + ext4) + # 5 times the disk quota: the test runs up to 4 workers + # concurrently; this makes sure they can't get spurious failures + # from running out of disk space + fallocate -l 320M ~/boxfs.img + mkfs.ext4 -O quota ~/boxfs.img + sudo mount -o loop,usrquota ~/boxfs.img /var/lib/isolate + ;; + tmpfs) + sudo mount -o usrquota -t tmpfs tmpfs /var/lib/isolate + ;; + *) + echo "Bad TEST_QUOTAS value (expected ext4 or tmpfs)" >&2 + exit 1 + ;; + esac +fi + cmsRunFunctionalTests -v --coverage codecov/functionaltests.xml FUNC=$? diff --git a/docker/docker-compose.test.yml b/docker/docker-compose.test.yml index 72048c2410..b12d12abf4 100644 --- a/docker/docker-compose.test.yml +++ b/docker/docker-compose.test.yml @@ -14,6 +14,7 @@ services: # - https://github.com/pytest-dev/pytest/issues/7443 # - https://github.com/actions/runner/issues/241 PYTEST_ADDOPTS: --color=yes + TEST_QUOTAS: ${TEST_QUOTAS-ext4} volumes: - "../codecov:/home/cmsuser/src/codecov" privileged: true diff --git a/docs/Running CMS.rst b/docs/Running CMS.rst index 9138ca4b53..90369f002c 100644 --- a/docs/Running CMS.rst +++ b/docs/Running CMS.rst @@ -61,8 +61,6 @@ These files are a pretty good starting point if you want to try CMS. There are s * you must change the connection string given in ``database``; this usually means to change username, password and database with the ones you chose before; -* if you are running low on disk space, you may want to make sure ``keep_sandbox`` is set to ``false``; - If you are organizing a real contest, you must also change ``secret_key`` to a random key (the admin interface will suggest one if you visit it when ``secret_key`` is the default). You will also need to think about how to distribute your services and change ``core_services`` accordingly. Finally, you should change the ranking section of :file:`cms.toml`, and :file:`cms_ranking.toml`, using non-trivial username and password. .. warning::