Repository navigation
Read commit-graph, midx and pack index integers without pointer casts - #7441
Open
vera-rubin wants to merge 2 commits into
Open
vera-rubin wants to merge 2 commits into
vera-rubin wants to merge 2 commits into
Conversation
Add helpers that read a big-endian integer from a byte buffer by composing it from its bytes, as git's own get_be32() and get_be64() do. Unlike `ntohl(*(uint32_t *)p)`, they make no assumption about the alignment of `p`. Compilers recognize the pattern and emit a single load and byte swap where the platform allows unaligned loads.
The commit-graph, multi-pack-index and pack index readers read 32-bit fields by casting a pointer into the file data to `uint32_t *` (or to a header struct) and dereferencing it, and keep the fanout tables as `const uint32_t *`. That is only valid when the data is suitably aligned. The parsers check chunk offsets against the file size but not their alignment, and git_commit_graph_file_parse() and git_midx_parse() take any buffer; the fuzzers skip four input bytes just to keep theirs aligned. Read these fields with git__get_be32() and git__get_be64() instead, keep the fanout tables as byte pointers, and copy the file headers into a local struct, as index.c already does for index entries. The parsed values are unchanged. Add tests that parse a commit-graph and a multi-pack-index from an odd address. Without this change they fail under the UBSan CI job.
This branch has not been deployed
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
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.
These readers read on-disk integers with
ntohl(*(uint32_t *)p)and keep the fanout tables asconst uint32_t *into file data. That assumes 4-byte alignment, which nothing checks; the fuzzers skip four bytes to stay aligned.This adds
git__get_be32()/git__get_be64()(byte composition, like git'sget_be32()), uses them throughout those readers, and copies file headers into a local struct asindex.cdoes.I left
bloom_filter_indexesalone since #7357 builds on it.If it lands, we may submit it to Google's Patch Rewards program, and we'd be glad to share any reward with the maintainers.
— ChipLab