Skip to content

Commit

Permalink
colblk: safeguard against Go pointer rule violations in DataBlockRewr…
Browse files Browse the repository at this point in the history
…iter

Go pointer rules require that a Go pointer always point to a byte within a
valid allocation. The DataBlockRewriter previously would allocate buffers for
keys exactly sized for the largest user key contained within a block. If one
were to use pointer arithmetic to compute an exclusive pointer to byte beyond
the key, this pointer would violate Go's pointer rules.

As far as I can tell, we don't do this today because only PrefixBytesIter
performs pointer arithmetic that might violate this by computing a pointer to
the beginning of the suffix. This is only problematic if there is no suffix,
but suffix-rewriting always requires a suffix.

To be safe, we allocate an extra byte.
  • Loading branch information
jbowens committed Oct 1, 2024
1 parent 1f53fae commit 34dd769
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
15 changes: 11 additions & 4 deletions sstable/colblk/data_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,10 +638,14 @@ func (rw *DataBlockRewriter) RewriteSuffixes(
if err = rw.iter.Init(&rw.reader, rw.keySeeker, nil, block.IterTransforms{}); err != nil {
return base.InternalKey{}, base.InternalKey{}, nil, err
}
if cap(rw.prefixBytesIter.buf) < int(rw.reader.maximumKeyLength) {
rw.prefixBytesIter.buf = make([]byte, rw.reader.maximumKeyLength)

// Allocate a keyIter buffer that's large enough to hold the largest user
// key in the block with 1 byte to spare (so that pointer arithmetic is
// never pointing beyond the allocation, which would violate Go rules).
if cap(rw.prefixBytesIter.buf) < int(rw.reader.maximumKeyLength)+1 {
rw.prefixBytesIter.buf = make([]byte, rw.reader.maximumKeyLength+1)
}
if newMax := int(rw.reader.maximumKeyLength) - len(from) + len(to); cap(rw.keyBuf) < newMax {
if newMax := int(rw.reader.maximumKeyLength) - len(from) + len(to) + 1; cap(rw.keyBuf) < newMax {
rw.keyBuf = make([]byte, newMax)
}

Expand Down Expand Up @@ -703,7 +707,10 @@ type DataBlockReader struct {
isObsolete Bitmap
// maximumKeyLength is the maximum length of a user key in the block.
// Iterators may use it to allocate a sufficiently large buffer up front,
// and elide size checks during iteration.
// and elide size checks during iteration. Note that iterators should add +1
// to the key length to ensure pointer arithmetric that computes a pointer
// to the tail of the key does not point to memory beyond the allocation
// (prohibited by Go pointer rules).
maximumKeyLength uint32
}

Expand Down
13 changes: 13 additions & 0 deletions sstable/testdata/rewriter_v5
Original file line number Diff line number Diff line change
Expand Up @@ -231,3 +231,16 @@ scan-range-key
a-b:{(#1,RANGEKEYSET,@123)}
b-c:{(#1,RANGEKEYSET,@123)}
c-d:{(#1,RANGEKEYSET,@123)}

build block-size=1 index-block-size=1 filter
a.SET.1:a
b.SET.1:b
c.SET.1:c
----
point: [a#1,SET-c#1,SET]
seqnums: [1-1]

rewrite from= to=@123 block-size=1 index-block-size=1 filter
----
point: [a@123#1,SET-c@123#1,SET]
seqnums: [1-1]

0 comments on commit 34dd769

Please sign in to comment.