Skip to content

Commit 1833715

Browse files
acatangiualexandruag
authored andcommitted
jailer: sanitize process on startup
To avoid leaking any inherited FDs into Firecracker, iterate through the whole FD table (`sysconf(_SC_OPEN_MAX)` is used to get the table size) and close everything except IN, OUT and ERR. Signed-off-by: Adrian Catangiu <[email protected]>
1 parent 3ab2705 commit 1833715

File tree

2 files changed

+16
-18
lines changed

2 files changed

+16
-18
lines changed

docs/jailer.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ jailer --id <id> \
4444
After starting, the Jailer goes through the following operations:
4545

4646
- Validate **all provided paths** and the VM `id`.
47-
- Close all open file descriptors unrelated to standard input.
47+
- Close all open file descriptors based on `sysconf(_SC_OPEN_MAX)` except input,
48+
output and error.
4849
- Open `/dev/kvm` as *RW*, and bind a Unix domain socket listener to
4950
`<chroot_base>/<exec_file_name>/<id>/api.socket`. `exec_file_name` is the
5051
last path component of `exec_file` (for example, that would be `firecracker`

jailer/src/lib.rs

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,18 @@ pub fn clap_app<'a, 'b>() -> App<'a, 'b> {
163163
)
164164
}
165165

166+
fn sanitize_process() {
167+
// First thing to do is make sure we don't keep any inherited FDs
168+
// other that IN, OUT and ERR.
169+
170+
// Safe because sysconf is available on all flavors of Linux.
171+
let fd_size = unsafe { libc::sysconf(libc::_SC_OPEN_MAX) } as i32;
172+
for i in 3..fd_size {
173+
// Safe because close() cannot fail when passed a valid parameter.
174+
unsafe { libc::close(i) };
175+
}
176+
}
177+
166178
fn open_dev_kvm() -> Result<i32> {
167179
// Safe because we use a constant null-terminated string and verify the result.
168180
let ret = unsafe { libc::open("/dev/kvm\0".as_ptr() as *const libc::c_char, libc::O_RDWR) };
@@ -185,23 +197,8 @@ pub fn run(args: ArgMatches, start_time_ms: u64) -> Result<()> {
185197
// TODO: can a malicious guest that takes over firecracker use its access to the KVM fd to
186198
// starve the host of resources? (cgroups should take care of that, but do they currently?)
187199

188-
if let Err(e) = open_dev_kvm() {
189-
if let Error::UnexpectedKvmFd(ret) = e {
190-
// The problem here might be that the customer did not close every fd > 2 before
191-
// invoking the jailer (and did not open files with the O_CLOEXEC flag to begin with).
192-
// Before failing, let's close all non stdio fds up to and including ret, and then try
193-
// one more time.
194-
for i in 3..=ret {
195-
// Safe becase we're passing a valid paramter.
196-
unsafe { libc::close(i) };
197-
}
198-
199-
// Maybe now we can get the desired fd number.
200-
open_dev_kvm()?;
201-
} else {
202-
return Err(e);
203-
}
204-
}
200+
sanitize_process();
201+
open_dev_kvm()?;
205202

206203
let env = Env::new(args, start_time_ms)?;
207204

0 commit comments

Comments
 (0)