Skip to content

Commit

Permalink
Paging Audit: Remove Bitwidth Check in x86 Page Parsing
Browse files Browse the repository at this point in the history
When parsing the x86 page table entries, the physical address
bits value collected is used to mask the page bytes. However,
this is unnecessary and incorrect as currently written.

The physical address bits value should be left shifted by 12 bits
to isolate the physical address bits. Even if we do this, the
masking is unnecessary because the physical address bits are
masked when reading the page table bytes.

- [x] Impacts functionality?
  - **Functionality** - Does the change ultimately impact how firmware functions?
  - Examples: Add a new library, publish a new PPI, update an algorithm, ...
- [ ] Impacts security?
  - **Security** - Does the change have a direct security impact on an application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
  - **Breaking change** - Will anyone consuming this change experience a break
    in build or boot behavior?
  - Examples: Add a new library class, move a module to a different repo, call
    a function in a new library class in a pre-existing module, ...
- [x] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
  - **Documentation** - Does the change contain explicit documentation additions
    outside direct code modifications (and comments)?
  - Examples: Update readme file, add feature readme file, link to documentation
    on an a separate Web page, ...

How This Was Tested

Tested on Q35 and SBSA by running the paging audit with various
memory protection profiles.

Tested on an SMM-enabled Surface device.

Integration Instructions

N/A
  • Loading branch information
TaylorBeebe committed Mar 19, 2024
1 parent 9754415 commit d023e93
Showing 1 changed file with 6 additions and 9 deletions.
15 changes: 6 additions & 9 deletions UefiTestingPkg/AuditTests/PagingAudit/Windows/BinaryParsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ def ParsePlatforminfofile(fileName):
def Parse4kPages(fileName):
num = 0
pages = []
addressbits = (1 << Globals.Bitwidth) - 1
logging.debug("-- Processing file '%s'..." % fileName)
ByteArray = ParseFileToBytes(fileName)
byteZeroIndex = 0
Expand All @@ -74,9 +73,9 @@ def Parse4kPages(fileName):
Present = ((ByteArray[byteZeroIndex + 0] & 0x1))
ReadWrite = ((ByteArray[byteZeroIndex + 0] & 0x2) >> 1)
User = ((ByteArray[byteZeroIndex + 0] & 0x4) >> 2)
PageTableBaseAddress = (((((ByteArray[byteZeroIndex + 1] & 0xF0) >> 4)) + (ByteArray[byteZeroIndex + 2] << 4) + (ByteArray[byteZeroIndex + 3] << 12) + (ByteArray[byteZeroIndex + 4] << 20) + (ByteArray[byteZeroIndex + 5] << 28) + ((ByteArray[byteZeroIndex + 6] & 0xF) << 36) << 12) & addressbits)
PageTableBaseAddress = ((((ByteArray[byteZeroIndex + 1] & 0xF0) >> 4) + (ByteArray[byteZeroIndex + 2] << 4) + (ByteArray[byteZeroIndex + 3] << 12) + (ByteArray[byteZeroIndex + 4] << 20) + (ByteArray[byteZeroIndex + 5] << 28) + ((ByteArray[byteZeroIndex + 6] & 0xF) << 36)) << 12)
Nx = ((ByteArray[byteZeroIndex + 7] & 0x80) >> 7)

logging.debug("4KB Page: 0x%s. Present: %d. ReadWrite: %d. User: %d. PageTableBaseAddress: %s" % (BytesToHexString(ByteArray[byteZeroIndex : byteZeroIndex + 8]), Present, ReadWrite, User, hex(PageTableBaseAddress)))
byteZeroIndex += 8
num += 1
pages.append(MemoryRange("PTEntry", "4k", Present, ReadWrite, Nx, 1, User, (PageTableBaseAddress)))
Expand Down Expand Up @@ -105,7 +104,6 @@ def Parse4kPages(fileName):
def Parse2mPages(fileName):
num = 0
pages = []
addressbits = (1 << Globals.Bitwidth) - 1
logging.debug("-- Processing file '%s'..." % fileName)
ByteArray = ParseFileToBytes(fileName)
byteZeroIndex = 0
Expand All @@ -118,9 +116,9 @@ def Parse2mPages(fileName):
ReadWrite = ((ByteArray[byteZeroIndex + 0] & 0x2) >> 1)
MustBe1 = ((ByteArray[byteZeroIndex + 0] & 0x80) >> 7)
User = ((ByteArray[byteZeroIndex + 0] & 0x4) >> 2)
PageTableBaseAddress = (((((ByteArray[byteZeroIndex + 2] & 0xE0) >> 5)) + (ByteArray[byteZeroIndex + 3] << 3) + (ByteArray[byteZeroIndex + 4] << 11) + (ByteArray[byteZeroIndex + 5] << 19) + ((ByteArray[byteZeroIndex + 6] & 0xF) << 27) << 21) & addressbits)
PageTableBaseAddress = ((((ByteArray[byteZeroIndex + 2] & 0xE0) >> 5) + (ByteArray[byteZeroIndex + 3] << 3) + (ByteArray[byteZeroIndex + 4] << 11) + (ByteArray[byteZeroIndex + 5] << 19) + ((ByteArray[byteZeroIndex + 6] & 0xF) << 27)) << 21)
Nx = ((ByteArray[byteZeroIndex + 7] & 0x80) >> 7)

logging.debug("2MB Page: 0x%s. Present: %d. ReadWrite: %d. MustBe1: %d. User: %d. PageTableBaseAddress: %s" % (BytesToHexString(ByteArray[byteZeroIndex : byteZeroIndex + 8]), Present, ReadWrite, MustBe1, User, hex(PageTableBaseAddress)))
byteZeroIndex += 8
num += 1
pages.append(MemoryRange("PTEntry", "2m", Present, ReadWrite, Nx, MustBe1, User, (PageTableBaseAddress)))
Expand Down Expand Up @@ -149,7 +147,6 @@ def Parse2mPages(fileName):
def Parse1gPages(fileName):
num = 0
pages = []
addressbits = (1 << Globals.Bitwidth) - 1
logging.debug("-- Processing file '%s'..." % fileName)
ByteArray = ParseFileToBytes(fileName)
byteZeroIndex = 0
Expand All @@ -162,9 +159,9 @@ def Parse1gPages(fileName):
ReadWrite = ((ByteArray[byteZeroIndex + 0] & 0x2) >> 1)
MustBe1 = ((ByteArray[byteZeroIndex + 0] & 0x80) >> 7)
User = ((ByteArray[byteZeroIndex + 0] & 0x4) >> 2)
PageTableBaseAddress = (((((ByteArray[byteZeroIndex + 3] & 0xC0) >> 6)) + (ByteArray[byteZeroIndex + 4] << 2) + (ByteArray[byteZeroIndex + 5] << 10) + ((ByteArray[byteZeroIndex + 6] & 0xF) << 18) << 30) & addressbits) # shift and address bits
PageTableBaseAddress = ((((ByteArray[byteZeroIndex + 3] & 0xC0) >> 6) + (ByteArray[byteZeroIndex + 4] << 2) + (ByteArray[byteZeroIndex + 5] << 10) + ((ByteArray[byteZeroIndex + 6] & 0xF) << 18)) << 30) # shift and address bits
Nx = ((ByteArray[byteZeroIndex + 7] & 0x80) >> 7)

logging.debug("1GB Page: 0x%s. Present: %d. ReadWrite: %d. MustBe1: %d. User: %d. PageTableBaseAddress: %s" % (BytesToHexString(ByteArray[byteZeroIndex : byteZeroIndex + 8]), Present, ReadWrite, MustBe1, User, hex(PageTableBaseAddress)))
byteZeroIndex += 8
pages.append(MemoryRange("PTEntry", "1g", Present, ReadWrite, Nx, MustBe1, User, PageTableBaseAddress))
num += 1
Expand Down

0 comments on commit d023e93

Please sign in to comment.