Skip to content

Commit

Permalink
Add extra 7 bytes to buffers passed to Util::SliceInt64FromBytes
Browse files Browse the repository at this point in the history
This function may access up to 7 bytes beyond the intended number of
bytes to be read, so add the extra allocation to avoid undefined
behavior.

Add a comment to Util::SliceInt64FromBytes stating this requirement
explicitly.
  • Loading branch information
rostislav authored and hoffmang9 committed Oct 23, 2020
1 parent f9a1363 commit f51fef6
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 22 deletions.
2 changes: 1 addition & 1 deletion src/calculate_bucket.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class F1Calculator {
uint8_t enc_key[32];
size_t buf_blocks = cdiv(k << kBatchSizes, kF1BlockSizeBits) + 1;
this->k_ = k;
this->buf_ = new uint8_t[buf_blocks * kF1BlockSizeBits / 8];
this->buf_ = new uint8_t[buf_blocks * kF1BlockSizeBits / 8 + 7];

// First byte is 1, the index of this table
enc_key[0] = 1;
Expand Down
2 changes: 1 addition & 1 deletion src/phase1.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void* phase1_thread(THREADDATA* ptd)
// writing results to the right table.
uint64_t left_buf_entries = 5000 + (uint64_t)((1.1)*(globals.stripe_size));
uint64_t right_buf_entries = 5000 + (uint64_t)((1.1)*(globals.stripe_size));
uint8_t* right_writer_buf = new uint8_t[right_buf_entries * right_entry_size_bytes]();
uint8_t* right_writer_buf = new uint8_t[right_buf_entries * right_entry_size_bytes + 7]();
uint8_t* left_writer_buf = new uint8_t[left_buf_entries * compressed_entry_size_bytes]();

FxCalculator f(k, table_index + 1);
Expand Down
2 changes: 1 addition & 1 deletion src/phase2.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ std::vector<uint64_t> RunPhase2(
uint64_t right_reader_buf_entries = sort_manager_buf_size / right_entry_size_bytes;
uint64_t left_writer_buf_entries = left_writer_buf_size / left_entry_size_bytes;
uint64_t right_writer_buf_entries = other_buf_sizes / right_entry_size_bytes;
uint64_t left_reader_buf_entries = other_buf_sizes / left_entry_size_bytes - 10;
uint64_t left_reader_buf_entries = other_buf_sizes / left_entry_size_bytes;
uint64_t left_reader_count = 0;
uint64_t right_reader_count = 0;
uint64_t left_writer_count = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/phase3.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ Phase3Results RunPhase3(
uint8_t *right_writer_buf = &(memory[sort_manager_buf_size]);
uint8_t *right_reader_buf = &(memory[sort_manager_buf_size + right_writer_buf_size]);
uint64_t left_reader_buf_entries = sort_manager_buf_size / left_entry_size_bytes;
uint64_t right_reader_buf_entries = right_reader_buf_size / right_entry_size_bytes - 10;
uint64_t right_reader_buf_entries = right_reader_buf_size / right_entry_size_bytes;
uint64_t left_reader_count = 0;
uint64_t right_reader_count = 0;
uint64_t total_r_entries = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/plotter_disk.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ class DiskPlotter {
assert(id_len == kIdLen);

// Memory to be used for sorting and buffers
std::unique_ptr<uint8_t[]> memory(new uint8_t[memory_size]);
std::unique_ptr<uint8_t[]> memory(new uint8_t[memory_size + 7]);

std::cout << std::endl
<< "Starting phase 1/4: Forward Propagation into tmp files... "
Expand Down
6 changes: 3 additions & 3 deletions src/prover_disk.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,9 @@ class DiskProver {
disk_file.seekg(table_begin_pointers[table_index] + (park_size_bits / 8) * park_index);

// This is the checkpoint at the beginning of the park
uint16_t line_point_size_bits = EntrySizes::CalculateLinePointSize(k) * 8;
uint8_t* line_point_bin = new uint8_t[line_point_size_bits / 8];
disk_file.read(reinterpret_cast<char*>(line_point_bin), line_point_size_bits / 8);
uint16_t line_point_size = EntrySizes::CalculateLinePointSize(k);
uint8_t* line_point_bin = new uint8_t[line_point_size + 7];
disk_file.read(reinterpret_cast<char*>(line_point_bin), line_point_size);
uint128_t line_point = Util::SliceInt128FromBytes(line_point_bin, 0, k * 2);

// Reads EPP stubs
Expand Down
2 changes: 1 addition & 1 deletion src/sort_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class SortManager {
this->final_position_start = 0;
this->final_position_end = 0;
this->next_bucket_to_sort = 0;
this->entry_buf = new uint8_t[entry_size]();
this->entry_buf = new uint8_t[entry_size + 7]();
}

inline void AddToCache(const Bits &entry)
Expand Down
11 changes: 7 additions & 4 deletions src/util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,13 @@ namespace Util {
return count;
}

// bytes points to a big-endian 64 bit value (possibly truncated, if
// start_bit + num_bits < 64). Returns the integer that starts at start_bit
// that is num_bits long (as a native-endian integer).
// Note: requires start_bit % 8 + num_bits <= 64
// 'bytes' points to a big-endian 64 bit value (possibly truncated, if
// (start_bit % 8 + num_bits > 64)). Returns the integer that starts at
// 'start_bit' that is 'num_bits' long (as a native-endian integer).
//
// Note: requires that 8 bytes after the first sliced byte are addressable
// (regardless of 'num_bits'). In practice it can be ensured by allocating
// extra 7 bytes to all memory buffers passed to this function.
inline uint64_t SliceInt64FromBytes(
const uint8_t *bytes,
uint32_t start_bit,
Expand Down
18 changes: 9 additions & 9 deletions tests/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ static uint128_t to_uint128(uint64_t hi, uint64_t lo) { return (uint128_t)hi <<

TEST_CASE("SliceInt64FromBytes 1 bit")
{
const uint8_t bytes[] = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9};
const uint8_t bytes[9 + 7] = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9};

// since we interpret the first 64 bits (8 bytes) as big endian, the
// first byte is 0x01
Expand Down Expand Up @@ -84,7 +84,7 @@ TEST_CASE("SliceInt64FromBytes 1 bit")

TEST_CASE("SliceInt64FromBytes 8 bits")
{
const uint8_t bytes[] = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9};
const uint8_t bytes[9 + 7] = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9};

// since we interpret the first 64 bits (8 bytes) as big endian, the
// first byte is 0x01
Expand Down Expand Up @@ -114,7 +114,7 @@ TEST_CASE("SliceInt64FromBytes 8 bits")

TEST_CASE("SliceInt64FromBytes 24 bits")
{
const uint8_t bytes[] = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9};
const uint8_t bytes[9 + 7] = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9};

// since we interpret the first 64 bits (8 bytes) as big endian, the
// first byte is 0x01
Expand All @@ -130,7 +130,7 @@ TEST_CASE("SliceInt64FromBytes 24 bits")

TEST_CASE("SliceInt64FromBytesFull")
{
const uint8_t bytes[] = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9};
const uint8_t bytes[9 + 7] = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9};

// since we interpret the first 64 bits (8 bytes) as big endian, the
// first byte is 0x01
Expand All @@ -149,11 +149,11 @@ TEST_CASE("Util")
{
SECTION("Increment and decrement")
{
uint8_t bytes[3] = {45, 172, 225};
uint8_t bytes[3 + 7] = {45, 172, 225};
REQUIRE(Util::SliceInt64FromBytes(bytes, 2, 19) == 374172);
uint8_t bytes2[1] = {213};
uint8_t bytes2[1 + 7] = {213};
REQUIRE(Util::SliceInt64FromBytes(bytes2, 1, 5) == 21);
uint8_t bytes3[17] = {1, 2, 3, 4, 5, 6, 7, 255, 255, 10, 11, 12, 13, 14, 15, 16, 255};
uint8_t bytes3[17 + 7] = {1, 2, 3, 4, 5, 6, 7, 255, 255, 10, 11, 12, 13, 14, 15, 16, 255};
uint128_t int3 = to_uint128(0x01020304050607ff, 0xff0a0b0c0d0e0f10);
REQUIRE(Util::SliceInt64FromBytes(bytes3, 64, 64) == (uint64_t)int3);
REQUIRE(Util::SliceInt64FromBytes(bytes3, 0, 60) == (uint64_t)(int3 >> 68));
Expand Down Expand Up @@ -648,12 +648,12 @@ TEST_CASE("Sort on disk")
SECTION("ExtractNum")
{
for (int i = 0; i < 15 * 8 - 5; i++) {
uint8_t buf[15];
uint8_t buf[15 + 7];
Bits((uint128_t)27 << i, 15 * 8).ToBytes(buf);

REQUIRE(Util::ExtractNum(buf, 15, 15 * 8 - 4 - i, 3) == 5);
}
uint8_t buf[16];
uint8_t buf[16 + 7];
Bits((uint128_t)27 << 5, 128).ToBytes(buf);
REQUIRE(Util::ExtractNum(buf, 16, 100, 200) == 864);
}
Expand Down

0 comments on commit f51fef6

Please sign in to comment.