Repository navigation
Fix parsing of some AMD client ROMs - #87
Merged
Merged
Conversation
Some client images carry a 64 MB flash, such as the Lenovo ThinkPad Z13 Gen 2 (BIOS N41ET63W), whose BIOS L2 directory points at 0x3f40100. With the ROM capped at 32 MB that entry fails with "File offset overflows ROM bounds!" and the whole image fails to parse. The size taken from the input file stays the default. Only when parsing the first page hits that overflow, and 64 MB fits after the ROM offset, is the ROM parsed again as 64 MB. Picking 64 MB from the file size alone would skip a 32 MB ROM that starts past offset 0 in a 64 MB file, and would let compressed entries of a 32 MB ROM in a bigger file run into the bytes after it. Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
In some images the 0x48/0x4a entries of the $PSP directory point at the $PL2 directory itself rather than at a header holding its offset, for example the Lenovo ThinkPad L13 Gen 4 (BIOS R26ET54W). Reading an offset from inside the directory then gives garbage, the L2 directory is skipped as empty, and every file in it is missing from the output. A directory reached this way inherits the parent's generation, which can be None. update_zen_generation now assigns in that case instead of failing with a TypeError when the same directory is reached again with a generation. The generation read from an indirection header gets its own name, so it does not leak into a direct pointer later in the same directory. Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
A BIOS directory entry of type 0x5c in the Lenovo ThinkPad L13 Gen 4 (BIOS R26ET54W) holds a plain PE image, so the header's size field at 0x6c is text (0x6b20796e). The file then grew past the ROM and parsing failed later with an assertion in NestedBuffer. Raise File.ParseError instead, which skips the entry with a warning. Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
In the Lenovo ThinkPad T14 Gen 7 AMD (BIOS R38SG07W) every 0x4a entry of the $PSP directory leads to a slot header whose location is an APCB rather than a $PL2 directory. from_offset raised Directory.ParseError for the unknown magic, which aborted the whole parse, so none of the directories or files were listed. Skip such a target with a warning, the same way empty entries already are. The $PL2 behind the 0x48 slot headers still parses, including its DRTM TA. Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
coreboot's amdfwtool writes the APOB NV entries of SoCs up to Cezanne and Mendocino as an x86 physical address (SPI_ROM_BASE plus the flash offset) with entry address mode 0, inside a BIOS directory with address mode 1. The entry address mode was only honoured in mode 2 and 3 directories, so the offset 0xff020000 was read as a flash offset and the ROM failed with "File offset overflows ROM bounds!", e.g. the StarBook Mk VI AMD firmware on LVFS. Treat an entry with address mode 0 and an offset of 0xff000000 or more as a physical address in a mode 1 directory. No flash offset reaches that range. Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
iwanicki92
changed the title
Member
|
Reopening to retrigger CI with the updated workflow (actions/checkout pwn-request fix). |
Metrics
|
| Metric | Value |
|---|---|
| Files Processed | 68 |
| Stdout Lines | 14,965 |
| Stderr Lines | 164 |
| Total Lines | 15,129 |
| Error Ratio | 1.08% |
PSPTool python library metrics
| Status | Count | Percentage |
|---|---|---|
| ✓ Successful | 68 | 100.0% |
| ✗ Failed | 0 | 0.0% |
| Total | 68 | 100% |
All files parsed successfully!
Member
|
Great work, thanks a lot, especially for the regression testing! |
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
2 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.
Five fixes for AMD client images that failed to parse or lost files, found on public BIOS downloads:
0x3f40100hit "File offset overflows ROM bounds!". Retry as a 64 MB ROM only when that happens and 64 MB fits.0x48/0x4aentries point straight at the$PL2directory, so the L2 was read as empty. Follow such pointers, and letupdate_zen_generationhandle aNonegeneration.0x5cBIOS entry is a plain PE, its header size is text and ran past the ROM into aNestedBufferassertion. RaiseFile.ParseErrorso the entry is skipped with a warning..SC1capsule): every0x4aentry leads to a slot header whose target is an APCB, not a$PL2.from_offsetraisedDirectory.ParseErrorfor the unknown magic and the whole parse aborted. Skip such a target with a warning, the same way empty entries are skipped. The$PL2behind the0x48slot headers still parses.amdfwtoolwrites the APOB NV entry (0x63) as the x86 address0xff020000with entry address mode 0 in a$BL2with address mode 1, which hit "File offset overflows ROM bounds!".amdfwtooldoes this for every SoC up to Cezanne and Mendocino. Treat an entry with address mode 0 and an offset of0xff000000or more as a physical address in such a directory.Each fix has a unit test in
tests/unit/test_rom_layouts.py. I also compared-E -joutput with master on 949 distinct images: the A285/A485/T495 fixtures plus public BIOS downloads from Lenovo, HP, Dell, MSI, Gigabyte and ASRock Rack (not added to the repo). No image loses an entry or fails that master parses.