fix(attest): increase heap for c lib#187
Closed
haitaohuang wants to merge 1 commit into
Closed
Conversation
haitaohuang
force-pushed
the
upstream/pr2-attestation-fixes
branch
from
6d94d51 to
e37d194
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent heap exhaustion in the C-based quote verification path by increasing the private attestation heap size. It also updates MigTD migration wire parsing logic (for vmcall-raw + policy_v2) to support an optional “short vs full” payload form.
Changes:
- Increase
ATTEST_HEAP_SIZEfrom 512 KiB to 2 MiB with additional rationale in comments. - Add
vmcall-raw + policy_v2-specific parsing helpers forMigtdMigrationInformation(optional init TDINFO tail). - Update
Cargo.lockto include an additionalspinversion dependency entry.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/migtd/src/migration/mod.rs |
Adds policy_v2 + vmcall-raw defaulting and byte-parsing logic for migration info payloads with an optional tail. |
src/attestation/src/attest.rs |
Raises the attestation C-lib heap allocation size to reduce verifier heap exhaustion. |
Cargo.lock |
Records updated dependency resolution (adds spin 0.10.0 alongside existing versions). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+183
to
+194
| #[cfg(all(feature = "vmcall-raw", feature = "policy_v2"))] | ||
| impl Default for MigtdMigrationInformation { | ||
| fn default() -> Self { | ||
| Self { | ||
| mig_request_id: 0, | ||
| migration_source: 0, | ||
| has_init_data: 0, | ||
| _reserved: [0; 6], | ||
| target_td_uuid: [0; 4], | ||
| binding_handle: 0, | ||
| init_td_info: [0u8; TD_INFO_SIZE], | ||
| } |
Comment on lines
+218
to
+226
| pub fn read_from_bytes( | ||
| data_length: u32, | ||
| payload: &[u8], | ||
| ) -> core::result::Result<Self, MigrationResult> { | ||
| let short_size = MIGTD_MIGRATION_INFO_HEADER_SIZE as u32; | ||
| let full_size = core::mem::size_of::<Self>() as u32; | ||
| if data_length != short_size && data_length != full_size { | ||
| return Err(MigrationResult::InvalidParameter); | ||
| } |
Comment on lines
+30
to
40
| // NOTE: bumped from 0x80000 (512 KiB) to 0x200000 (2 MiB). | ||
| // Each verify_quote_integrity_ex() call allocates from this private heap and | ||
| // the dlmalloc heap does not move sbrk down (limited defragmentation after | ||
| // each free() calls) to original position after the call returns. With | ||
| // LOCAL_TCB_INFO caching removed (commit 3c44ea9), each migration now does | ||
| // up to 4 verify_quote calls in the same MigTD process (loopback), which can | ||
| // exhaust the 512 KiB heap at 3rd call due to statics and fragmentation in | ||
| // heap, and trigger an internal abort (#UD) inside the C verifier lib with | ||
| // no error code returned. | ||
| const ATTEST_HEAP_SIZE: usize = 0x200000; | ||
|
|
| // NOTE: bumped from 0x80000 (512 KiB) to 0x200000 (2 MiB). | ||
| // Each verify_quote_integrity_ex() call allocates from this private heap and | ||
| // the dlmalloc heap does not move sbrk down (limited defragmentation after | ||
| // each free() calls) to original position after the call returns. With |
4 tasks
- Raise C-lib heap from 512 KiB to 2 MiB for loopback migration (4 verify_quote calls exhaust the old heap due to fragmentation) - Add init_td_info field and read_from_bytes for policy_v2 wire format Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
haitaohuang
force-pushed
the
upstream/pr2-attestation-fixes
branch
from
e37d194 to
3c3b398
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
After removing the init tcb cache, in a loopback migration scenario, we need call verify_quote_integrity_ex 4 times for each migration. That would cause heap exhaustion inside the c lib.
Raise the size of heap