Fix lookup and overflow in simplefs_ext_search#86
Open
RoyWFHuang wants to merge 1 commit into
Open
Conversation
The final hit check used 'iblock < end_len' instead of 'iblock < end_block + end_len', so a block in any extent not starting at block 0 was missed and returned as the unused 'boundary' slot (read as a hole, or overwritten on write). Also return -1 when the extent array is full, instead of the out-of-bounds 'boundary' == SIMPLEFS_MAX_EXTENTS. Close sysprog21#76
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.
The final hit check used 'iblock < end_len' instead of 'iblock < end_block + end_len', so a block in any extent not starting at block 0 was missed and returned as the unused 'boundary' slot (read as a hole, or overwritten on write).
Also, return -1 when the extent array is full, instead of returning the out-of-bounds, SIMPLEFS_MAX_EXTENTS
Actually, this code is never executed: simplefs_ext_search is only reachable through simplefs_file_get_block, which is wired into the page-cache aops (readahead/readpage/write_begin). Since read()/write() on master don't use the page cache at all, the buggy function is effectively dead code.
So if we really want to test this code, we can use "readahead" to reach it.
Here is an example:
Close #76
Summary by cubic
Fix extent lookup and overflow in
simplefs_ext_searchto correctly match blocks within extents and avoid out-of-bounds indices. Prevents false holes and unintended overwrites. Fixes #76.iblock < end_block + end_lento check final hit, instead ofiblock < end_len.-1when the extent array is full, instead ofSIMPLEFS_MAX_EXTENTS.Written for commit 8895361. Summary will update on new commits.