-
Notifications
You must be signed in to change notification settings - Fork 93
fix(jailer): replace hardcoded FD limits with dynamic closure strategies #406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,160 @@ | |||||||||||
| //! | ||||||||||||
| //! Only the async-signal-safe `close_inherited_fds_raw()` is used, | ||||||||||||
| //! called from the `pre_exec` hook before exec(). | ||||||||||||
| //! | ||||||||||||
| //! ## Strategy (in order of preference) | ||||||||||||
| //! | ||||||||||||
| //! 1. **Linux 5.9+**: `close_range(first_fd, ~0U, 0)` — O(1), kernel closes all. | ||||||||||||
| //! 2. **Linux < 5.9**: Enumerate `/proc/self/fd` via raw `getdents64` syscall | ||||||||||||
| //! (no memory allocation) and close only open FDs. Falls back to brute-force | ||||||||||||
| //! with a dynamic upper bound from `getrlimit(RLIMIT_NOFILE)`. | ||||||||||||
| //! 3. **macOS**: Brute-force close with dynamic upper bound from | ||||||||||||
| //! `getrlimit(RLIMIT_NOFILE)` instead of a hardcoded limit. | ||||||||||||
|
|
||||||||||||
| /// Query the soft limit for `RLIMIT_NOFILE`. Async-signal-safe. | ||||||||||||
| /// | ||||||||||||
| /// Returns the soft limit (current max open files), or a safe default | ||||||||||||
| /// if the query fails. `getrlimit` is async-signal-safe per POSIX. | ||||||||||||
| /// | ||||||||||||
| /// Note: Cannot reuse `rlimit::get_rlimit()` because it returns `io::Error` | ||||||||||||
| /// which heap-allocates (not async-signal-safe for pre_exec context). | ||||||||||||
| fn get_max_fd() -> i32 { | ||||||||||||
| let mut rlim = libc::rlimit { | ||||||||||||
| rlim_cur: 0, | ||||||||||||
| rlim_max: 0, | ||||||||||||
| }; | ||||||||||||
| // SAFETY: getrlimit is async-signal-safe per POSIX. rlim is a valid stack-allocated struct. | ||||||||||||
| let result = unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, &mut rlim) }; | ||||||||||||
| if result == 0 && rlim.rlim_cur > 0 { | ||||||||||||
| if rlim.rlim_cur < i32::MAX as u64 { | ||||||||||||
| rlim.rlim_cur as i32 | ||||||||||||
| } else { | ||||||||||||
| // rlim_cur is RLIM_INFINITY or very large; cap to a safe maximum. | ||||||||||||
| // 1048576 (2^20) matches Linux's default /proc/sys/fs/nr_open. | ||||||||||||
| 1_048_576 | ||||||||||||
| } | ||||||||||||
| } else { | ||||||||||||
| // getrlimit failed or returned 0; safe default per POSIX OPEN_MAX | ||||||||||||
| 1024 | ||||||||||||
|
Comment on lines
+42
to
+43
|
||||||||||||
| // getrlimit failed or returned 0; safe default per POSIX OPEN_MAX | |
| 1024 | |
| // getrlimit failed or returned 0; use the same conservative cap to | |
| // ensure we still attempt to close a wide range of FDs. | |
| 1_048_576 |
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
close_fds_via_proc currently returns true even if getdents64 fails (e.g., syscall returns -1 for EINTR/EBADF). That would skip the brute-force fallback and can leave FDs open. Consider treating nread < 0 (and also malformed d_reclen conditions) as a failure: close dir_fd and return false so brute_force_close_fds() runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_max_fd()usesif rlim.rlim_cur < i32::MAX as u64which incorrectly treats an exacti32::MAXsoft limit as “too large” and caps it to 1,048,576. This looks like an off-by-one comparison; use<=if the intent is “fits in i32”.