[WIP] rebase of feature/secret-hiding onto main #5379
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
roypat
force-pushed
the
sh_rebased
branch
6 times, most recently
from
a8a29ab to
27d21c8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5379 +/- ##
==========================================
- Coverage 82.31% 81.44% -0.87%
==========================================
Files 265 265
Lines 30645 31354 +709
==========================================
+ Hits 25224 25535 +311
- Misses 5421 5819 +398
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
roypat
force-pushed
the
sh_rebased
branch
2 times, most recently
from
b023ede to
5f73808
Compare
kalyazin
reviewed
Aug 14, 2025
| .read_event() | ||
| .expect("Failed to read uffd_msg") | ||
| .expect("uffd_msg not ready"); | ||
| let Some(event) = uffd_handler.read_event().expect("Failed to read uffd_msg") else { |
There was a problem hiding this comment.
this one I have no idea
roypat
force-pushed
the
sh_rebased
branch
3 times, most recently
from
e70f9c5 to
b0acfa6
Compare
Creating a script to build and install a modified kernel with patches applied. Signed-off-by: Jack Thomson <[email protected]>
Adding a new integration test to assert that the kernel build script will succeed. Signed-off-by: Jack Thomson <[email protected]>
Adding the secret hiding kernel as a default for the buildkite pipeline, this will mean that PR's made against the branch will now be run with the new secret hiding enabled amis. Some tests have been marked to skip as they are kernel dependent so while we are compiling our kernel in CI these could change again. Signed-off-by: Jack Thomson <[email protected]>
To make it easier to track the upstream kernels which may change as we rebase, let's mark kernels newer than 6.12 as next for now to make dashboarding easier. Signed-off-by: Jack Thomson <[email protected]>
Addressing a comment to move away from dir stacks in our install scripts. We now store the start directly before we move the build directory and cd back to that explicitly. Signed-off-by: Jack Thomson <[email protected]>
Run the kernel build as part of our nightly tests so we can monitor it's success. Signed-off-by: Jack Thomson <[email protected]>
Add all required linux host kernel patches required for secret hiding. These are: - v17 of guest_memfd direct map support [1] - Direct map removal patches - make kvm_clock work with direct map removed guest_memfd - v2 of KVM_USERFAULT patches [2] - support for UFFDIO_CONTINUE in guest_memfd VMAs - support for write(2) syscall for guest_memfd [1]: https://lore.kernel.org/kvm/[email protected]/T/#m729a1f14fbbccdd66ed5fe434ff3a9d46b055dec [2]: https://lore.kernel.org/kvm/[email protected]/ emoval series to apply on top of v17. Rebase all the other series. Signed-off-by: Patrick Roy <[email protected]>
The patches are in the `patches` subdirectory of `hiding_ci`, so if only patches were added, then the check of "any files with parent directory `hiding_ci`" would be false, and the CI step for testing the build of patches wouldn't actually run. Fix this by updating the check to be "any files where any parent directory is `hiding_ci`", which will also catch patches. Reported-by: Jack Thomson <[email protected]> Signed-off-by: Patrick Roy <[email protected]>
Update the build script to allow us to install the secret hidden kernels onto Amazon Linux 2023 instances. We have to as part of this include a script to download and install ena drivers for the instance to allow us to boot. Signed-off-by: Jack Thomson <[email protected]>
The output from the build in x86 is archived so updated the script to support installing either output type from the build Signed-off-by: Jack Thomson <[email protected]>
Add an 'apt update' before `apt install`. Otherwise, we might hold an old view of the package versions and installation might fail. Signed-off-by: Babis Chalios <[email protected]>
This lint forbids using `..Default::default()` in struct initializers after all fields have already been initialized, but this is a useful pattern if you know you want to add more fields to a struct in a future PR without needing to touch a ton of initializers in unittests again (_heavy foreshadowing_). So silence the paperclip. Signed-off-by: Patrick Roy <[email protected]>
There's no need to test this through VmResources when it can be tested in isolation. Also, everytime I touch MachineConfig I get confsued by where the hell the tests are, cuz not only are they in a different module, they're also one directory level away. So move the tests into machine_config.rs, where it makes sense to have them. Signed-off-by: Patrick Roy <[email protected]>
With secret freedom, direct accesses to guest memory from the context of the host kernel are no longer possible. This particularly means that we cannot pass pointers to guest memory to the host kernel anymore (at least if the host kernel tries to GUP them). For these scenarios, introduce a utility decorator struct `MaybeBounce` that can optionally do indirect read and write syscalls on guest memory by first memcpy-ing to firecracker userspace, and passing a pointer to firecracker heap memory into the kernel instead. Signed-off-by: Patrick Roy <[email protected]>
This is particularly useful for virtio devices, where on-demand allocation of bounce buffers leads to sever performance impacts (~80%) in synthetic throughput tests. Additionally, for virtio devices we can know approximately what the optimal size of a statically allocated bounce buffer is. Allocate bounce buffers on the heap, as trying to even temporarily place a 65k bounce buffer on the stack can lead to stack overflow errors. Signed-off-by: Patrick Roy <[email protected]>
Add support to our virtio devices to allow userspace bounce buffering of virtio buffers. This is an alternative to swiotlb. Don't implement it for vhost-user-blk and for virtio-block with async engine, because I have no idea how that would even work. Signed-off-by: Patrick Roy <[email protected]>
If the CI artifacts dont contain old firecracker releases, still succeed at setting them up after downloading them. Signed-off-by: Patrick Roy <[email protected]>
Add a test that we can boot VMs and initrds with secret freedom enabled. Signed-off-by: Patrick Roy <[email protected]>
Since we load the kernel using bounce buffers now, it will give us false-positives. Signed-off-by: Patrick Roy <[email protected]>
Move from Ubuntu to AL2023 for the secret hiding testing to bring it inline with the other kernels We had to add some more kernel config overrides. The amazon linux default kernel didn't have CRYPTO_HW enabled, this is required as a dependency for AMD_SEV. Signed-off-by: Jack Thomson <[email protected]>
The install script on amazon linux isn't storing the .config in our boot directory by default. This is causing our spectre checker script which relies on the config. Updated our script to move this if it has't been done so already. Signed-off-by: Jack Thomson <[email protected]>
We are not using the .lore/.mbox options, and I dont see us doing so again in the future either. Signed-off-by: Patrick Roy <[email protected]>
This is needed because if guest_memfd is used to back guest memory, vCPU fault notifications are delivered via the UFFD UDS socket. Signed-off-by: Nikita Kalyazin <[email protected]>
It is used by Secret-Free-enabled UFFD handlers to disable vCPU fault notifications from the kernel. Signed-off-by: Nikita Kalyazin <[email protected]>
Accept receiving 3 fds instead of 1, where fds[1] is guest_memfd and fds[2] is userfault bitmap memfd. Also handle the FaultRequest message over the UDS socket by calling a new callback in the Runtime and sending a FaultReply. Co-authored-by: Patrick Roy <[email protected]> Signed-off-by: Patrick Roy <[email protected]> Signed-off-by: Nikita Kalyazin <[email protected]>
There are two ways a UFFD handler receives a fault notification if Secret Fredom is enabled (which is inferred from 3 fds sent by Firecracker instead of 1): - a VMM- or KVM-triggered fault is delivered via a minor UFFD fault event. The handler is supposed to respond to it via memcpying the content of the page (if the page hasn't already been populated) followed by a UFFDIO_CONTINUE call. - a vCPU-triggered fault is delievered via a FaultRequest message on the UDS socket. The handler is supposed to reply with a pwrite64 call on the guest_memfd to populate the page followed by a FaultReply message on the UDS socket. In both cases, the handler also needs to clear the bit in the userfault bitmap at the corresponding offset in order to stop further fault notifications for the same page. UFFD handlers use the userfault bitmap for two purposes: - communicate to the kernel whether a fault at the corresponding guest_memfd offset will cause a VM exit - keep track of pages that have already been populated in order to avoid overwriting the content of the page that is already initialised. Signed-off-by: Nikita Kalyazin <[email protected]>
These are used for communication of page faults between Firecracker and a UFFD handler. Signed-off-by: Nikita Kalyazin <[email protected]>
If configured, userfault bitmap is registered with KVM and controls whether KVM will exit to userspace on a fault of the corresponding page. We are going to allocate the bitmap in a memfd in Firecracker, set bits for all pages to request notifications for vCPU faults and send it to the UFFD handler to delegate clearing the bits as pages get populated. Since the KVM userfault patches are still in review, set_user_memory_region2 is not aware of the userfault flag and the userfault bitmap address in its input structure. Define it in Firecracker code temporarily. Signed-off-by: Nikita Kalyazin <[email protected]>
This is needed to instruct the kernel to exit to userspace when a vCPU fault occurs and the corresponding bit in the userfault bitmap is set. The userfault bitmap is allocated in a memfd by Firecracker and sent to the UFFD handler. This also sends 3 fds to the UFFD handler in the handshake: - UFFD (original) - guest_memfd: for the handler to be able to populate guest memory - userfault bitmap memfd: for the handler to be able to disable exits to userspace for the pages that have already been populated Signed-off-by: Nikita Kalyazin <[email protected]>
This is because vCPUs reason in GPAs while the secret-free UFFD protocol is guest_memfd-offset-based. Note that offset_to_gpa is not used yet, but will likely be needed to support async PF to pass the GPA to a new ioctl when notifying KVM of a fault resolution. Signed-off-by: Nikita Kalyazin <[email protected]>
It contains two parts: - external: between the VMM thread and the UFFD handler - internal: between vCPUs and the VMM thread An outline of the workflow: - When a vCPU fault occurs, vCPU exits to userspace - The vCPU thread sends sends the exit syndrome in the vCPU to VMM channel and writes to the eventfd - The VMM thread forwards the syndrome to the UFFD handler via the UDS socket - The UFFD handler populates the page, clears the corresponding bit in the userfault bitmap and sends a reply to Firecracker - The VMM thread receives the reply and updates a vCPU condvar to notify the vCPU that the fault has been resolved - The vCPU resumes execution Note that as a result of this change, an ability to exit the VM gracefully is lost (at least on x86). In the existing implementation, the VMM thread initiated an exit if an event was read from the eventfd, but no VcpuResponse::Exited responses were read for unknown reason. Since the exit_evt eventfd is now also used by vCPUs to notify the VMM thread of the VM exits caused by pagefaults, this situation (an eventfd event, but response in the channel) can occur also because we have read all VcpuResponse::Userfault in response to the previous eventfd event. Signed-off-by: Nikita Kalyazin <[email protected]>
In a regular VM, we mmap the memory snapshot file and supply the address in the KVM memory slot. In Secret Free VMs, we provide guest_memfd in the memory slot instead. There is no way we can restore a Secret Free VM from a file, unless we prepopulate the guest_memfd with the file content, which is inefficient and is not practically useful. Signed-off-by: Nikita Kalyazin <[email protected]>
This includes both functional and performance tests. Signed-off-by: Nikita Kalyazin <[email protected]>
Do not add a balloon device to a Secret Free VM as it is not currently supported. Signed-off-by: Nikita Kalyazin <[email protected]>
This is because the error type has changed due the implementation of snapshot restore support for Secret Free VMs. Signed-off-by: Nikita Kalyazin <[email protected]>
Graceful shutdown is currently broken on x86_64. Signed-off-by: Nikita Kalyazin <[email protected]>
Writing to the noturbo sysfs immediately locks up the entire instance, so stop doing this for now. Signed-off-by: Patrick Roy <[email protected]>
Without this, the script will ask for user input and get stuck if run unattended. Signed-off-by: Patrick Roy <[email protected]>
Started seeing the below failure in test_population_latency: thread 'main' panicked at .../uffd/fault_all_handler.rs:41:18: uffd_msg not ready note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace I am not entierly sure how this can happen, because the read from the uffd is supposed to be blocking, but maybe it's a weird interaction with the fault-all behavior (e.g. there was a uffd event queues, but because we faulted everything it got cancelled again?), so let's just try going back to read(2) if we dont read anything. Signed-off-by: Patrick Roy <[email protected]>
Currently, we often get stuck with the problem where something in the host kernel breaks that causes functional tests to fail, but we cannot update the patch series from which the host kernel gets built, because functional tests are failing. Break this cyclic dependency by simply not running functional tests when updating only the patch series (as they dont test the updated kernel anyway. Signed-off-by: Patrick Roy <[email protected]>
Return errors up the stack instead of panicking. Signed-off-by: Nikita Kalyazin <[email protected]>
Return None if file_offset() is None instead. Signed-off-by: Nikita Kalyazin <[email protected]>
This is to make sure that we always write the entire FaultRequest message even if the syscall was interrupted. Signed-off-by: Nikita Kalyazin <[email protected]>
Make sure we continue reading the FaultReply if the syscall was interrupted. Signed-off-by: Nikita Kalyazin <[email protected]>
Get rid of the expect by using indexing. Signed-off-by: Nikita Kalyazin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
...
Reason
...
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.