Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract PDR section of Flash #36

Merged
merged 1 commit into from
Feb 22, 2016
Merged

Conversation

timevortex
Copy link
Contributor

FlashRegionSectionType already parses PdrBase and PdrLimit. This bit of the code just makes sure that FlashDescriptor uses that information to actually extract that region out.

I've tested this against a BIOS that has PDR section and it seems to work fine:

pparth@doctor:/tmp/uefi-firmware-parser/regions$ ls -al
drwxr-x---  3 pparth eng     4096 Feb 18 16:58 region-bios
-rw-r-----  1 pparth eng 11468800 Feb 18 16:58 region-bios.fd
-rw-r-----  1 pparth eng     8192 Feb 18 16:58 region-gbe.fd
-rw-r-----  1 pparth eng  5287936 Feb 18 16:58 region-me.fd
-rw-r-----  1 pparth eng     8192 Feb 18 16:58 region-pdr.fd
pparth@doctor:~/tmp/uefi-firmware-parser/regions$ 

8192 bytes for the PDR region is correct - confirmed by extracting using UEFIExtract:

pparth@doctor:/tmp/uefiextract/bios.dump/2 PDR region$ ls -al
-rw-r----- 1 pparth eng 8192 Feb 18 17:01 body.bin
-rw-r----- 1 pparth eng   49 Feb 18 17:01 info.txt
pparth@doctor:/tmp/uefiextract/bios.dump/2 PDR region$ cat info.txt 
Type: Region
Subtype: PDR
Full size: 2000h (8192)

Diff confirms that the outputs between UEFIExtract and uefi_firmware are identical :)

FlashRegionSectionType already parses PdrBase and PdrLimit. This bit of the code just makes sure that FlashDescriptor uses that information to extract that region out.
@NikolajSchlej
Copy link

You should also add reserved regions, which are typical for SPS systems. Take SPSFullME.bin file from this archive as example. Thanks to @lordkag for finding it.

@timevortex
Copy link
Contributor Author

So I had a look around the UEFI Spec and I can't seem to find anything that specs out the reserved regions. Does anyone know where these are documented or is this just something that SPS Systems use that is out of spec?

I think this should be a separate feature request because this would require changing the structs definitions in here: https://github.com/theopolis/uefi-firmware-parser/tree/master/uefi_firmware/structs

@theopolis The test seems to be failing because of SSH hanging at key-auth, is this something you can fix?

@theopolis
Copy link
Owner

@timevortex the tests attempt to pull from a private repo using an encrypted ssh private key. The key is encrypted using Travis' API with the assumption that their build hosts will be able to decrypt. There's no sensitive information in this private repo but it does host vendor-copyrighted UEFI/firmware distributions so I cannot rehost them, publicly, legally.

It looks like (from a small bit of debugging), that Travis is only decryption the key for branched to this repo, as opposed to anyone's branch an the appropriate PR.

@theopolis
Copy link
Owner

I actually don't mind that the private repo "uefi-firmware-samples" can be "leaked". I can embed a deploy key, that's not encrypted, into the .travis.yml configuration then if people want to download the example samples there's at least a pretty involved effort requirement.

@timevortex
Copy link
Contributor Author

Thanks for re-running the test!

Inconsistency parsing (flash/Latitude-E6410-A10.flash): expected 1186 objects, found: 1187
This 'may' be expected if this change improves the object discovery/parsing logic.

This seems OK given that we're probably extracting the extra PDR region for this flash now. @theopolis, is this ready for merging or is there anything else?

"base": pdr_base,
"limit": pdr_limit,
})
pdr_region.process()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually the return boolean within .process() should be the return off all the inter-processing. But since the flash processing already violates this we should be OK to merge.

@theopolis
Copy link
Owner

Sure, I'll take care of the tests in a follow up PR. Thanks @timevortex!

theopolis pushed a commit that referenced this pull request Feb 22, 2016
Extract PDR section of Flash
@theopolis theopolis merged commit e6f122a into theopolis:master Feb 22, 2016
@timevortex
Copy link
Contributor Author

Cheers! Happy to help.

I'm happy to contribute more as well. This was a simple change but for other request I think I want make sure you're onboard as well. e.g. #35 probably requires some major code restructuring. I'm happy to do it but I don't want to send you a PR that's huge and unexpected!

@theopolis
Copy link
Owner

Yeah that'd be great! I'm happy to test offline if needed. I'm also sending some emails to figure out if I can just open the samples repository. There is a script in that private repo that tracks the expected number of "objects" discovered in each-- that's used to make sure no code changes remove functionality. But if we're adding more parsing then those should be updated inline to reflect the new (larger) counts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants