fix buffer overflow in DiskAccess::read_exact_into
#524
Conversation
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
DiskAccess::read_exact_into did not work correctly if current_offset wasn't aligned to 512 bytes. For example, if self.current_offset is 3 and len is 1024: - end_addr will be 3+1024=1027 - start_lba will be 3/512=0 - end_lba (1027-1)=2 read_exact_into would then read 3 (!) sectors with lbas 0, 1, and 2 even though the buffer can only hold 2 sectors (1024 bytes). To fix this, only allow seeking to the start of a sector. DiskAccess::read_exact_into works correctly if the offset is a multiple of the sector size. There were few uses of DiskAccess::seek that didn't seek to the start of a sector. All those uses then use DiskAccess::read_exact to read data at the offset. Unlike DiskAccess::read_exact_into, DiskAccess::read_exact can already handle non-aligned offsets. To fix the bad seek calls, we turn read_exact into a combined seek+offset function that works for non-aligned offsets.
This generates smaller binaries.
Now that we fixed the buffer overflow and found another way of generating smaller binaries, we no longer need to use the old symbol mangling scheme. The new symbol mangling was never really at fault. All it did was shuffle some of the symbols around resulting in a larger binary. We just got unlucky there.
phil-opp
approved these changes
Dec 1, 2025
Member
phil-opp
left a comment
phil-opp
left a comment
There was a problem hiding this comment.
Oh wow, thanks a lot for getting to the bottom of this!
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.
This PR fixes a buffer overflow (see commit message for details) and adjusts the optimization level to generate smaller binaries.
The buffer overflow became noticeable on newer nightlies, but it's always been around. With the old mangling scheme, the linker placed the symbols in a different order where the buffer overflow happened to not cause any harm. The order generated by the linker with the new scheme made the buffer overflow noticeable.
Cc @adoerr