From d7ddb3e3bb80259a1081492c9f83be4a15f18f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Mon, 3 Jul 2023 17:59:48 +0200 Subject: [PATCH] jailer: remove a TODO and clarify docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove a TODO that suggests doing something that could have security implications. Clarify the wording so it is clear why we do it this way and what the implications of hardlinking are. Signed-off-by: Pablo Barbáchano --- docs/jailer.md | 3 ++- src/jailer/src/env.rs | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/jailer.md b/docs/jailer.md index 3025701260e..aae3edf9a09 100644 --- a/docs/jailer.md +++ b/docs/jailer.md @@ -108,7 +108,8 @@ After starting, the Jailer goes through the following operations: for `/usr/bin/firecracker`). Nothing is done if the path already exists (it should not, since `id` is supposed to be unique). - Copy `exec_file` to - `///root/`. + `///root/`. This ensures the + new process will not share memory with any other Firecracker process. - Set resource bounds for current process and its children through `--resource-limit` argument, by calling `setrlimit()` system call with the specific resource argument. If no limits are provided, the jailer bounds diff --git a/src/jailer/src/env.rs b/src/jailer/src/env.rs index 5a41a183390..6d1a4215600 100644 --- a/src/jailer/src/env.rs +++ b/src/jailer/src/env.rs @@ -416,8 +416,13 @@ impl Env { // a new PathBuf, with something like chroot_dir.join(exec_file_name) ?! self.chroot_dir.push(exec_file_name); - // TODO: hard link instead of copy? This would save up disk space, but hard linking is - // not always possible :( + // We do a copy instead of a hard-link for 2 reasons + // 1. hard-linking is not possible if the file is in another device + // 2. while hardlinking would save up disk space and also memory by + // sharing parts of the Firecracker binary (like the executable .text + // section), this latter part is not desirable in Firecracker's + // threat model. Copying prevents 2 Firecracker processes from + // sharing memory. fs::copy(&self.exec_file_path, &self.chroot_dir).map_err(|err| { Error::Copy(self.exec_file_path.clone(), self.chroot_dir.clone(), err) })?;