Skip to content

Commit

Permalink
jailer: remove a TODO and clarify docs
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
pb8o committed Jul 4, 2023
1 parent d9cec89 commit d7ddb3e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
3 changes: 2 additions & 1 deletion docs/jailer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
`<chroot_base>/<exec_file_name>/<id>/root/<exec_file_name>`.
`<chroot_base>/<exec_file_name>/<id>/root/<exec_file_name>`. 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
Expand Down
9 changes: 7 additions & 2 deletions src/jailer/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})?;
Expand Down

0 comments on commit d7ddb3e

Please sign in to comment.