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

EDK2UEFIVarStore serialization broken on AAVMF #9

Closed
kukrimate opened this issue Jan 23, 2024 · 7 comments
Closed

EDK2UEFIVarStore serialization broken on AAVMF #9

kukrimate opened this issue Jan 23, 2024 · 7 comments

Comments

@kukrimate
Copy link
Contributor

Both reading and writing an AAVMF variable store using EDK2UEFIVarStore seems to work.

However after serialization, the new varstore is no longer readable to AAVMF firmware itself.

I am investigating this issue in more detail and will update this bug with more information.

@kukrimate
Copy link
Contributor Author

This seems to be happening when self.length (the firmware volume length from the header) doesn't match the file length.

Our AAVMF varstores are padded to 64M, but only the first 4M is marked as a valid FV in the header. Despite this the bytes() function puts self.filelen into the header which stops AAVMF from understanding the varstore on the next boot.

I am not sure what's strictly correct solution here:

  • either it should fail noting that filelen doesnt match FVH.FvLength (then external users can deal with the padding)
  • or use the FVH.FvLength as the next FV header's length and only pad the file to filelen

@agraf
Copy link
Contributor

agraf commented Jan 23, 2024

What if you just set filelen to the FVH.FvLength on __init__? If you merely modify an edk2 varstore, the rest should then fall in place, no?

@kukrimate
Copy link
Contributor Author

@agraf I guess that's one solution for producing a correct header, it would just mean bytes() wouldn't return the same length of data that was originally passed to the constructor.

I suppose that isn't strictly an issue, client code would just need to pad the new varstore if it cares about the size.

@agraf
Copy link
Contributor

agraf commented Jan 24, 2024

I'd prefer it that way around, yes. Would you mind to cook up a patch?

@kukrimate
Copy link
Contributor Author

Please see PR #10

@kukrimate
Copy link
Contributor Author

kukrimate commented Jan 25, 2024

Another update concerning my issue:

  • During the testing for the header length fix, I am sometimes seeing further variable storage corruption seemingly due to the firmware's FaultTolerantWrite driver trying to reclaim space and proceed to corrupt the varstore following a modification by python-uefivars.
  • I've noticed the original firmware volume has a fault-tolerant write working block header, but the new one does not. Maybe this should be produced by the library when creating edk2 format varstores?

I'll do more investigation to see how can I make the library produce working variable stores for AAVMF reliably, just wanted to update that the header fix is likely not all that's required.

@agraf agraf closed this as completed in e07bd0a Jan 25, 2024
agraf added a commit that referenced this issue Jan 25, 2024
edk2.py: Use length from firmware volume header (Fixes #9)
@kukrimate
Copy link
Contributor Author

@agraf I've done some testing and it isn't the working block header, FTW can happily create a new one. The problem was that the FVH header was parsed incorrectly and the blockmap was hard coded. See PR: #11

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

No branches or pull requests

2 participants