Replace most NVRAM parsers with ones generated from KaitaiStruct definitions#412
Merged
NikolajSchlej merged 16 commits intonew_enginefrom Mar 13, 2025
Merged
Replace most NVRAM parsers with ones generated from KaitaiStruct definitions#412NikolajSchlej merged 16 commits intonew_enginefrom
NikolajSchlej merged 16 commits intonew_enginefrom
Conversation
Collaborator
Author
|
Will have to make a bit more changes here for the sake of optimization, turns out trying every parser on every byte got rather slow if it creates a ton of C++ objects and destroys those objects immediately. Update: done, ready for review. |
NikolajSchlej
force-pushed
the
dev/kty_nvram
branch
from
b62d915 to
db66a0d
Compare
Collaborator
Author
|
Local cross-testing with about 30 different samples of Insyde H2O and Phoenix SCT based FWs (AMI NVAR parser is not affected by this change) showed no unexpected results.
|
vit9696
reviewed
Mar 13, 2025
Contributor
vit9696
left a comment
vit9696
left a comment
There was a problem hiding this comment.
Made a brief review. I mostly focused on useability and safety issues, trusting you and kaitai on the format matter.
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.
Back in 2023 the initial attempt to fuzz the parsing engine with LibFuzzer and HongFuzz quickly failed because the fuzzer found an issue with our NVRAM parsers requiring backtracking to parse things properly, and that effectively dead-looped them on way too many possible inputs to count.
As an effort to get rid of that, and rewrite manual parsers on something more declarative and saner, I've started to rewrite NVRAM parsers in KaitaiStruct, and the first one (AMI NVAR) had been in use since about A62, and proven to work better than expected. Then I had to return to the US and restart my full-day employment, so all work on UEFITool had to be postponed. Until this month that is, and this PR is the result of it.
@vit9696, please review, as I do not expect this new code to change drastically any time soon. I will be doing cross-tests between A69 and this branch, trying to catch any possible offset errors and other things that Kaitai's codegen can not cover.