Skip to content

fix(pci): BAR sizing and MMIO bounds from security audit#949

Open
MichalTarnacki wants to merge 1 commit into
intel:mainfrom
MichalTarnacki:fix/impl-bugs-pci
Open

fix(pci): BAR sizing and MMIO bounds from security audit#949
MichalTarnacki wants to merge 1 commit into
intel:mainfrom
MichalTarnacki:fix/impl-bugs-pci

Conversation

@MichalTarnacki

Copy link
Copy Markdown
Contributor
# Assessed Severity Bug Call Chain / Location Fix Classification Disposition Reason
1 High PCI BAR raw VMM address stored when get_bar_size()==0 or IoSpace -> PciDevice::init Reject size==0 BARs; never store raw host BAR addr -- always route through alloc_mmio* or reject. true_positive
2 Medium get_bar_size: !(size&0xFFFFFFF0)+1 wraps u32->0 when host returns size in 1..=0xF -> PciDevice::get_bar_size Mask first, check ==0, then use wrapping_neg or explicit (!x).wrapping_add(1) with zero-check. weakness A hostile VMM supplying 0x1..=0xF would just result in size 0 and the BAR being treated as unimplemented — no actual exploit. The fix improves correctness and clarity but the old code was not exploitable in production.
3 Medium alloc_mmio64 lacks upper-bound check; init_mmio overlap check can wrap -> alloc_mmio64 Add MMIO64_START+MMIO64_SIZE bound; use checked_add in init_mmio overlap test. weakness The old code used plain + which in release mode wraps on overflow and in debug mode panics, but the operands come from trusted sources: MMIO constants are small compile-time values that cannot overflow u64, and the memory map is populated from TD-Shim HOBs during early trusted initialization with physical addresses limited to 52 bits on x86. No realistic input can trigger the overflow. The saturating_add change improves defensive coding style but does not fix an exploitable or reachable bug.

- reject size==0/IoSpace BARs, never store raw host BAR addr
- get_bar_size mask-first zero-check
- alloc_mmio64 upper-bound and checked_add overlap test

Signed-off-by: Michal Tarnacki <[email protected]>
Co-authored-by: GitHub Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant