Skip to content

Commit

Permalink
TDBStore: Must use work buffer with size of programming unit.
Browse files Browse the repository at this point in the history
In TDBStore::copy_record() we must prepare our content in
size of full programming unit. If we don't have big enough buffer
we end up writing garbage from our stack.

In most cases, this is prevent by usage of output buffer, but when
writing the record header, due the padding, we must write one full
programming unit.
  • Loading branch information
Seppo Takalo committed Nov 9, 2020
1 parent cdc2d45 commit c8426e8
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 16 deletions.
21 changes: 11 additions & 10 deletions storage/kvstore/source/TDBStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ typedef struct {
uint32_t crc;
} reserved_trailer_t;

static const uint32_t work_buf_size = 64;
static const uint32_t initial_crc = 0xFFFFFFFF;
static const uint32_t initial_max_keys = 16;

Expand Down Expand Up @@ -327,7 +326,7 @@ int TDBStore::read_record(uint8_t area, uint32_t offset, char *key,
user_key_ptr[key_size] = '\0';
} else {
dest_buf = _work_buf;
chunk_size = std::min(key_size, work_buf_size);
chunk_size = std::min(key_size, _prog_size);
}
} else {
// This means that we're on the data part
Expand All @@ -337,13 +336,13 @@ int TDBStore::read_record(uint8_t area, uint32_t offset, char *key,
// 3. After actual part is finished - read to work buffer
// 4. Copy data flag not set - read to work buffer
if (curr_data_offset < data_offset) {
chunk_size = std::min((size_t)work_buf_size, (size_t)(data_offset - curr_data_offset));
chunk_size = std::min((size_t)_prog_size, (size_t)(data_offset - curr_data_offset));
dest_buf = _work_buf;
} else if (copy_data && (curr_data_offset < data_offset + actual_data_size)) {
chunk_size = actual_data_size;
dest_buf = static_cast<uint8_t *>(data_buf);
} else {
chunk_size = std::min(work_buf_size, total_size);
chunk_size = std::min(_prog_size, total_size);
dest_buf = _work_buf;
}
}
Expand Down Expand Up @@ -824,17 +823,19 @@ int TDBStore::copy_record(uint8_t from_area, uint32_t from_offset, uint32_t to_o
uint32_t &to_next_offset)
{
int ret;
record_header_t header;
record_header_t *header = (record_header_t *) _work_buf;
uint32_t total_size;
uint16_t chunk_size;

ret = read_area(from_area, from_offset, sizeof(header), &header);
memset(_work_buf, 0, _prog_size);

ret = read_area(from_area, from_offset, sizeof(record_header_t), header);
if (ret) {
return ret;
}

total_size = align_up(sizeof(record_header_t), _prog_size) +
align_up(header.key_size + header.data_size, _prog_size);;
align_up(header->key_size + header->data_size, _prog_size);;


if (to_offset + total_size > _size) {
Expand All @@ -847,7 +848,7 @@ int TDBStore::copy_record(uint8_t from_area, uint32_t from_offset, uint32_t to_o
}

chunk_size = align_up(sizeof(record_header_t), _prog_size);
ret = write_area(1 - from_area, to_offset, chunk_size, &header);
ret = write_area(1 - from_area, to_offset, chunk_size, header);
if (ret) {
return ret;
}
Expand All @@ -857,7 +858,7 @@ int TDBStore::copy_record(uint8_t from_area, uint32_t from_offset, uint32_t to_o
total_size -= chunk_size;

while (total_size) {
chunk_size = std::min(total_size, work_buf_size);
chunk_size = std::min(total_size, _prog_size);
ret = read_area(from_area, from_offset, chunk_size, _work_buf);
if (ret) {
return ret;
Expand Down Expand Up @@ -1041,7 +1042,7 @@ int TDBStore::init()
}

_prog_size = _bd->get_program_size();
_work_buf = new uint8_t[work_buf_size];
_work_buf = new uint8_t[_prog_size];
_key_buf = new char[MAX_KEY_SIZE];
_inc_set_handle = new inc_set_handle_t;
memset(_inc_set_handle, 0, sizeof(inc_set_handle_t));
Expand Down
14 changes: 8 additions & 6 deletions storage/kvstore/tests/UNITTESTS/TDBStore/moduletest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@
#include "kvstore/TDBStore.h"
#include <stdlib.h>

#define BLOCK_SIZE (512)
#define BLOCK_SIZE (256)
#define DEVICE_SIZE (BLOCK_SIZE*200)

using namespace mbed;

class TDBStoreModuleTest : public testing::Test {
protected:
HeapBlockDevice heap{DEVICE_SIZE};
HeapBlockDevice heap{DEVICE_SIZE, BLOCK_SIZE};
FlashSimBlockDevice flash{&heap};
TDBStore tdb{&flash};

Expand Down Expand Up @@ -77,15 +77,17 @@ TEST_F(TDBStoreModuleTest, erased_set_get)

TEST_F(TDBStoreModuleTest, set_deinit_init_get)
{
char buf[100];
size_t size;
for (int i = 0; i < 100; ++i) {
EXPECT_EQ(tdb.set("key", "data", 5, 0), MBED_SUCCESS);
char str[11] = {0};
char buf[100] = {0};
int len = snprintf(str, 11, "data%d", i) + 1;
EXPECT_EQ(tdb.set("key", str, len, 0), MBED_SUCCESS);
EXPECT_EQ(tdb.deinit(), MBED_SUCCESS);
EXPECT_EQ(tdb.init(), MBED_SUCCESS);
EXPECT_EQ(tdb.get("key", buf, 100, &size), MBED_SUCCESS);
EXPECT_EQ(size, 5);
EXPECT_STREQ("data", buf);
EXPECT_EQ(size, len);
EXPECT_STREQ(str, buf);
EXPECT_EQ(tdb.remove("key"), MBED_SUCCESS);
}
}
Expand Down

0 comments on commit c8426e8

Please sign in to comment.