Reseed libc rng on snapshot-restore - #1667
Conversation
0dd83dd to
7b9f66b
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a new sandbox configuration knob, RngRestorePolicy, to control whether the guest libc PRNG state is preserved from a snapshot or reseeded when restoring/resuming from a snapshot. This fits into the host/guest snapshot/restore flow by using scratch bookkeeping to pass a reseed request from host to guest at restore time.
Changes:
- Add
RngRestorePolicytoSandboxConfigurationand thread it intoMultiUseSandboxso restore/from-snapshot can optionally request a libc PRNG reseed. - Add a scratch bookkeeping field (
SCRATCH_TOP_LIBC_RNG_SEED_OFFSET) plus guest-side logic to consume it and callsrand()before dispatching guest calls. - Add C-guest-based regression tests validating default preservation and optional reseeding behavior across
from_snapshotandrestore.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/tests/c_guests/c_simpleguest/main.c | Adds a NextRandom guest export (wrap + register) for testing libc RNG behavior. |
| src/hyperlight_host/src/sandbox/uninitialized_evolve.rs | Threads RNG restore policy into MultiUseSandbox construction. |
| src/hyperlight_host/src/sandbox/snapshot/file_tests.rs | Adds snapshot/restore tests for libc RNG preservation vs reseeding using the C simple guest. |
| src/hyperlight_host/src/sandbox/mod.rs | Re-exports RngRestorePolicy from the sandbox module. |
| src/hyperlight_host/src/sandbox/initialized_multi_use.rs | Implements host-side reseed requests on from_snapshot and restore, plus stores policy in the sandbox. |
| src/hyperlight_host/src/sandbox/config.rs | Introduces RngRestorePolicy and getters/setters on SandboxConfiguration. |
| src/hyperlight_host/src/mem/mgr.rs | Adds a scratch bookkeeping write helper for the libc RNG seed and clears it during bookkeeping updates. |
| src/hyperlight_guest/src/layout.rs | Exposes the guest virtual address of the libc RNG seed scratch slot. |
| src/hyperlight_guest_bin/src/lib.rs | Adds guest-side reseed logic (refresh_libc_rng) and updates initial srand seeding via a shared fold. |
| src/hyperlight_guest_bin/src/guest_function/call.rs | Calls refresh_libc_rng() on each internal dispatch (libc feature). |
| src/hyperlight_common/src/layout.rs | Defines the new scratch bookkeeping offset constant. |
| CHANGELOG.md | Documents the new configuration option in the unreleased changelog. |
I haven't looked at the implementation, but from the description this concerns me a bit. It also feels to me like "whether I need access to actual randomness" is more a semantic property of the guest being run than it is a property of the sandbox, so I wonder if it would make more sense to make it a (persisted) property of the snapshot that the sandbox honours. Snapshots which don't set the rng-needed flag could leave it out of their manifest for backwards compatibility. I also wonder whether or not there is really any benefit to the current rng initialisation that is shared between everyone. I could imagine that rather than looking at it as "do I need randomness re-initialised on snapshot restore or not", which might be somewhat opaque to users, the semantically simpler thing to consider, which is basically identical, is "do I need randomness at all or not". If we recast the API from that perspective, we could change to having a snapshot-level flag that switches between "not initialising randomness at all (/ideally also erroring if you try to use it)" and "initialising randomness on every restore". |
e092ea3 to
bd5c3e4
Compare
Agreed, I made it property of the snapshot now. And made it a break change which should be fine given we get this in before release
This would be nice, but how does this cover same/different rng for multiple sandboxes from the same snapshot? From each guest's perspective, they both could get randomness, but it might be the same as the other sandbox's |
My instinct would be to say that if you need randomness at all you should get reseeded randomness, and if you don't need it all you don't get anything ofc. So there is no case where 2 restores of the same snapshot can call rand() and think it works and get the same value. I don't think there is probably almost any case where you want that. Maybe if you do have randomness enabled, you could have host API to mock it with a fixed seed, but idk if that's useful or not. Either way, that would be a decision of the host. It seems like "do I need randomness" is a property of the code-being-run-in-the-sandbox, and changes its ABI to require a randomness reseed on snapshot restore, whereas "do I want to mock the randomness for a test" is a property of the host code, and does not change the ABI (it just means a fixed value is used on that reseed). |
Simplified this PR to its core essence, please see new updated PR description. |
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
Restoring snapshot now unconditionally reseeds libc rand() and random() to prevent multiple snapshots from sharing prng state.
For simplicity sake, decided against adding new config or policy regarding this, since this PR will fix the immediate issue. If we in the future want host to modify it, we can add it later. Users can also add guest functions to call srand manually if they wish.
Perf: adds unconditional ~7ns to restore path on host side on my local machine. We could store a cached ThreadRng handle on sandbox, which brings this down to ~2ns (avoid TLS lookup), but I didn't think this was worth it as it's already so fast, and most users don't care about randomness. On guest side,
srand()is just 1 direct memory store (negligible, a couple ns at most)Compatibility: old snapshot load fine but the guests will ignore the host's request to reseed. This is fine as #1674 will be a breaking change anyway.