From 3dbbe35b7f044bee101f03a916c76c2f8050e97b Mon Sep 17 00:00:00 2001 From: Riccardo Mancini Date: Fri, 28 Feb 2025 12:35:43 +0000 Subject: [PATCH 1/3] fix: do not build debug a default seccomp policy in debug binaries Rust 1.80.0 added a debug assertion that uses fcntl(F_GETFD) to ensure the fd is still valid when it gets dropped, which broke debug builds of firecracker. This made us rethink on whether we'd want any default seccomp policy in debug builds, and we decided that in most cases we don't need them and in some cases they get in the way of prororyping and debugging. This patch changes the default seccomp policy in debug builds to empty. Signed-off-by: Riccardo Mancini --- src/firecracker/build.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/firecracker/build.rs b/src/firecracker/build.rs index 87710b54fc4..595b1e0c00a 100644 --- a/src/firecracker/build.rs +++ b/src/firecracker/build.rs @@ -14,23 +14,34 @@ const SECCOMPILER_SRC_DIR: &str = "../seccompiler/src"; fn main() { // Target triple let target = std::env::var("TARGET").expect("Missing target."); + let debug: bool = std::env::var("DEBUG") + .expect("Missing debug.") + .parse() + .expect("Invalid env variable DEBUG"); let out_dir = std::env::var("OUT_DIR").expect("Missing build-level OUT_DIR."); // Target arch (x86_64 / aarch64) let target_arch = std::env::var("CARGO_CFG_TARGET_ARCH").expect("Missing target arch."); let seccomp_json_path = format!("{}/{}.json", JSON_DIR, target); - // If the current target doesn't have a default filter, use a default, empty filter. + // If the current target doesn't have a default filter, or if we're building a debug binary, + // use a default, empty filter. // This is to make sure that Firecracker builds even with libc toolchains for which we don't // provide a default filter. For example, GNU libc. - let seccomp_json_path = if Path::new(&seccomp_json_path).exists() { - seccomp_json_path - } else { + let seccomp_json_path = if debug { + println!( + "cargo:warning=Using empty default seccomp policy for debug builds: \ + `resources/seccomp/unimplemented.json`." + ); + format!("{}/unimplemented.json", JSON_DIR) + } else if !Path::new(&seccomp_json_path).exists() { println!( "cargo:warning=No default seccomp policy for target: {}. Defaulting to \ `resources/seccomp/unimplemented.json`.", target ); format!("{}/unimplemented.json", JSON_DIR) + } else { + seccomp_json_path }; // Retrigger the build script if the JSON file has changed. From 612c13aaa3b8fedc794915bc6702afe8d471c0bc Mon Sep 17 00:00:00 2001 From: Riccardo Mancini Date: Fri, 28 Feb 2025 13:34:39 +0000 Subject: [PATCH 2/3] doc(seccomp): mention that debug builds don't have a default seccomp Following the previous commit, this patch mentions in the docs that debug builds don't have a default seccomp policy and which different syscalls are present in debug builds versus release. Signed-off-by: Riccardo Mancini --- docs/seccomp.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/seccomp.md b/docs/seccomp.md index dac55006bc5..06c7fd92d76 100644 --- a/docs/seccomp.md +++ b/docs/seccomp.md @@ -11,8 +11,10 @@ follows: - API - right before launching the HTTP server; - VCPUs - right before executing guest code. -**Note**: On experimental GNU targets, there are no default seccomp filters -installed, since they are not intended for production use. +> [!WARNING] +> +> On debug binaries and experimental GNU targets, there are no default seccomp +> filters installed, since they are not intended for production use. Firecracker uses JSON files for expressing the filter rules and relies on the [seccompiler](seccompiler.md) tool for all the seccomp functionality. @@ -58,6 +60,12 @@ Potential use cases: - Users of experimentally-supported targets (like GNU libc builds) may be able to use this feature to implement seccomp filters without needing to have a custom build of Firecracker. +- Users of debug binaries who need to use a seccomp filter for any reason will + be able to use this feature to implement seccomp filters without needing to + have a custom build of Firecracker. Note: there may be some differences in + syscalls between `debug` and `release` builds. A non-comprehensive list is: + - `fcntl(F_GETFD)` is used by debug assertions to verify a dropped `fd` is + valid. - Faced with a _theoretical_ production issue, due to a syscall that was issued by the Firecracker process, but not allowed by the seccomp policy, one may use a custom filter in order to quickly mitigate the issue. This can speed up the From 9f307ae6042fd11f851922495d8b251dca3d2794 Mon Sep 17 00:00:00 2001 From: Riccardo Mancini Date: Mon, 3 Mar 2025 10:49:38 +0000 Subject: [PATCH 3/3] doc(changelog): add note about empty seccomp policy in debug builds This patch adds an entry to the "Fixed" changelog list mentioning that debug builds are now built with an empty default seccomp policy. Signed-off-by: Riccardo Mancini --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 673a1b7a88d..7c4ed83f012 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,9 @@ and this project adheres to - [#5046](https://github.com/firecracker-microvm/firecracker/pull/5046): Retry KVM_CREATE_VM on EINTR that occasionally happen on heavily loaded hosts to improve reliability of microVM creation. +- [#5052](https://github.com/firecracker-microvm/firecracker/pull/5052): Build + the empty seccomp policy as default for debug builds to avoid crashes on + syscalls introduced by debug assertions from Rust 1.80.0. ## [1.10.1]