feat: prohibit guests from surpassing fd limits#1247
feat: prohibit guests from surpassing fd limits#1247n0toose wants to merge 7 commits intohermit-os:mainfrom
Conversation
Co-authored-by: Panagiotis "Ivory" Vasilopoulos <git@n0toose.net>
- bump uhyve-interface to 0.2.0 - perform virtaddr -> physaddr conversions in hermit for read, write. - use adjust serial_port based on version of uhyve-interface used - Exit hypercalls should encode the exit code instead of a pointer to the integer. ExitArgs has been removed. - v2: add v2 support in src/stats.rs, futureproof - v2: use u64, i64 instead of usize, isize - v2: improve errno returns in read, write hypercalls with v1 compat - v2: remove packed where unnecessary - v2: return better error (codes) for write, read - v2: return length of write for stdout, stderr prints - adjust serial test for v2 uhyve-interface (v2) should now write to the rcx register before triggering an IoOut hypercall on systems the x86_64 architecture so as to allow for using pointers to 64-bit addresses (ports restrict us to 32-bit). This restriction should not apply for serial writes, although it also remains possible for the kernel to print out messages. Co-authored-by: Ellen Emilia Anna Zscheile <fogti+devel@ytrizja.de> Co-authored-by: Jonathan Klimt <jonathan.klimt@eonerc.rwth-aachen.de>
... which causes excessive slowdowns at scale.
In some cases, the host may return errors that 'Hermit' doesn't "know about". This warning intends to aid with the debugging process.
- write_to_fd test is now part of fs_tests - standard streams' (written to by write_to_fd) output is checked - generate_params now takes Option<PathBuf> as guest_path, as such a path may not always be necessary (like in the case of write_to_fd). A superfluous guest_path was also removed from mounts_test.
- This allows Uhyve to gracefully handle file descriptor exhaustion which can be caused by a busy host or a malicious guest. - Introduction of a serial test (which also implicitly tests whether the errno provided by the host is 'parsed' properly). Not complete, but still something. - Dynamic calculation of available files for the guest (as in, files used by Uhyve are not included in the 'files left' variable). Fixes hermit-os#897
|
No idea why the integration tests that run locally (after having close to an hour trying to "engineer" that) just don't feel like running on GitHub Actions right now. |
|
Catching and handling the errors (taken from Might be the cleaner solution, in particular because they aren't Linux-specific. |
|
Also, the amount of places where fd limits can be imposed is frightingly large: https://www.baeldung.com/linux/error-too-many-open-files . And Linux occasionally adds new ones. |
|
I didn't find polling lsof or similar to be a good idea and explicitly did not touch anything involving setting those limitations
Those were the codes I'd theoretically return, not part of the Hermit ABI.
One could do that (and I should've mentioned it earlier), but this limits the possibility of being able to define arbitrary limits (e.g. max open files, enforced by the kernel)—the idea would be useful for an "same process, many guests" approach later. This is not part of this PR, but that could change. It is in line with plans to decide whether errors cause crashes, or whether a host mount is read only. It might make sense to do both, but not at the same time, to ensure that there is some parity e.g. on macOS. After all, it's better if Uhyve doesn't treat the underlying guests differently depending on the host OS—but it is completely fair for Uhyve to provide some additional features depending on the host OS (i.e. Landlock) or configuration (e.g. tracing). |
|
It seems like the many methods effectively affect the same thing, like how Unless if I'm missing something (e.g. ulimit?), the primary thing that I don't do is use |
| criterion = "0.8" | ||
| home = "0.5" | ||
| regex = { version = "1.12.2", default-features = false, features = ["unicode-perl"] } | ||
| serial_test = "3.3" |
There was a problem hiding this comment.
- This is probably something for a separate PR
- I'm not decided if I prefer another dependency, or if we should just use a global Mutex here, similar to what is done in
build_hermit_bin
|
|
||
| [target.'cfg(target_os = "hermit")'.dependencies] | ||
| hermit = "0.12" | ||
| hermit = { path = "../../../hermit-rs/hermit" } |
|
I think on Linux it might make more sense to set The errno codes mentioned are those that would be emitted by syscalls (e.g. |
|
btw. Mac OSX also supports I don't think it makes sense to reinvent the wheel for stuff that can be done with standard UNIX utilities just fine. |
|
Consider using https://docs.rs/rlimit/ instead of hacking around with non-portable |
|
After a guest has started running, do we even open any additional file handles which would necessitate such a paranoid implementation? The main thing that we might want to fix is the handling of |
This has not been tested with rftrace, but the current implementation checks the current amount of open file descriptors while UhyveVm is being instantiated. The only negative aspect of this right now is that a refactor (e.g. closing the ELF header before UhyveFileMap is initialized) could break the logic.
TBD, thanks. Will leave this on the backburner until v2 is merged. |
which can be caused by a busy host or a malicious guest.
the errno provided by the host is 'parsed' properly). Not
complete, but still something.
files used by Uhyve are not included in the 'files left' variable).
Fixes #897
Depends on #1219
Currently Linux-only.