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.

Sign up for GitHub

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

Jump to bottom

sstable: Add bounds checking for virtual sstable iterators #2616

Merged
merged 1 commit into from
Jun 15, 2023
Merged

sstable: Add bounds checking for virtual sstable iterators #2616

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 32 additions & 14 deletions sstable/reader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,7 @@ type singleLevelIterator struct {
// iteration.
vState *virtualState
// endKeyInclusive is set to force the iterator to treat the upper field as
// inclusive while iterating instead of exclusive. This is used for virtual
// sstable iteration where the file bounds are inclusive.
// inclusive while iterating instead of exclusive.
endKeyInclusive bool
index blockIter
data blockIter
Expand Down Expand Up @@ -464,6 +463,24 @@ func (i *singleLevelIterator) init(
return nil
}

// Helper function to check if keys returned from iterator are within global and virtual bounds.
func (i *singleLevelIterator) maybeVerifyKey(
iKey *InternalKey, val base.LazyValue,
) (*InternalKey, base.LazyValue) {
// maybeVerify key is only used for virtual sstable iterators.
if invariants.Enabled && i.vState != nil && iKey != nil {
key := iKey.UserKey

uc, vuc := i.cmp(key, i.upper), i.cmp(key, i.vState.upper.UserKey)
lc, vlc := i.cmp(key, i.lower), i.cmp(key, i.vState.lower.UserKey)

if (i.vState.upper.IsExclusiveSentinel() && vuc == 0) || (!i.endKeyInclusive && uc == 0) || uc > 0 || vuc > 0 || lc < 0 || vlc < 0 {
panic(fmt.Sprintf("key: %s out of bounds of singleLevelIterator", key))
}
}
return iKey, val
}

// setupForCompaction sets up the singleLevelIterator for use with compactionIter.
// Currently, it skips readahead ramp-up. It should be called after init is called.
func (i *singleLevelIterator) setupForCompaction() {
Expand Down Expand Up @@ -931,8 +948,7 @@ func (i *singleLevelIterator) SeekPrefixGE(
key = i.lower
}
}
k, v := i.seekPrefixGE(prefix, key, flags, i.useFilter)
return k, v
return i.seekPrefixGE(prefix, key, flags, i.useFilter)
}

func (i *singleLevelIterator) seekPrefixGE(
Expand Down Expand Up @@ -996,7 +1012,7 @@ func (i *singleLevelIterator) seekPrefixGE(
i.boundsCmp = 0
i.positionedUsingLatestBounds = true
k, value = i.seekGEHelper(key, boundsCmp, flags)
return k, value
return i.maybeVerifyKey(k, value)
}

// virtualLast should only be called if i.vReader != nil.
Expand Down Expand Up @@ -1143,7 +1159,7 @@ func (i *singleLevelIterator) SeekLT(
// be chosen as "compleu". The SeekGE in the index block will then point
// us to the block containing "complexion". If this happens, we want the
// last key from the previous data block.
return i.skipBackward()
return i.maybeVerifyKey(i.skipBackward())
}

// First implements internalIterator.First, as documented in the pebble
Expand All @@ -1163,6 +1179,7 @@ func (i *singleLevelIterator) First() (*InternalKey, base.LazyValue) {
}
i.positionedUsingLatestBounds = true
i.maybeFilteredKeysSingleLevel = false

return i.firstInternal()
}

Expand Down Expand Up @@ -1375,8 +1392,9 @@ func (i *singleLevelIterator) NextPrefix(succKey []byte) (*InternalKey, base.Laz
return nil, base.LazyValue{}
}
}
return key, val
return i.maybeVerifyKey(key, val)
}

return i.skipForward()
}

Expand Down Expand Up @@ -1445,7 +1463,7 @@ func (i *singleLevelIterator) skipForward() (*InternalKey, base.LazyValue) {
return nil, base.LazyValue{}
}
}
return key, val
return i.maybeVerifyKey(key, val)
}
}
return nil, base.LazyValue{}
Expand Down Expand Up @@ -1488,7 +1506,7 @@ func (i *singleLevelIterator) skipBackward() (*InternalKey, base.LazyValue) {
i.exhaustedBounds = -1
return nil, base.LazyValue{}
}
return key, val
return i.maybeVerifyKey(key, val)
}
return nil, base.LazyValue{}
}
Expand Down Expand Up @@ -2324,7 +2342,7 @@ func (i *twoLevelIterator) SeekLT(
}
if result == loadBlockOK {
if ikey, val := i.singleLevelIterator.lastInternal(); ikey != nil {
return ikey, val
return i.maybeVerifyKey(ikey, val)
}
// Fall through to skipBackward since the singleLevelIterator did
// not have any blocks that satisfy the block interval
Expand All @@ -2338,7 +2356,7 @@ func (i *twoLevelIterator) SeekLT(
}
if result == loadBlockOK {
if ikey, val := i.singleLevelIterator.SeekLT(key, flags); ikey != nil {
return ikey, val
return i.maybeVerifyKey(ikey, val)
}
// Fall through to skipBackward since the singleLevelIterator did
// not have any blocks that satisfy the block interval
Expand Down Expand Up @@ -2519,7 +2537,7 @@ func (i *twoLevelIterator) NextPrefix(succKey []byte) (*InternalKey, base.LazyVa
}
}
} else if key, val := i.singleLevelIterator.SeekGE(succKey, base.SeekGEFlagsNone); key != nil {
return key, val
return i.maybeVerifyKey(key, val)
}
return i.skipForward()
}
Expand Down Expand Up @@ -2557,7 +2575,7 @@ func (i *twoLevelIterator) skipForward() (*InternalKey, base.LazyValue) {
}
if result == loadBlockOK {
if ikey, val := i.singleLevelIterator.firstInternal(); ikey != nil {
return ikey, val
return i.maybeVerifyKey(ikey, val)
}
// Next iteration will return if singleLevelIterator set
// exhaustedBounds = +1.
Expand Down Expand Up @@ -2598,7 +2616,7 @@ func (i *twoLevelIterator) skipBackward() (*InternalKey, base.LazyValue) {
}
if result == loadBlockOK {
if ikey, val := i.singleLevelIterator.lastInternal(); ikey != nil {
return ikey, val
return i.maybeVerifyKey(ikey, val)
}
// Next iteration will return if singleLevelIterator set
// exhaustedBounds = -1.
Expand Down