Skip to content

Commit

Permalink
Fixing page pruning / compaction crash (#15)
Browse files Browse the repository at this point in the history
Issue: the heap AM has a function which automatically compacts pages
when certain conditions are met. When this happens, it moves the
tuples around within the page. As encryption uses the offset of tuples
for decrypting them, this results in garbage data and possible crashes.

Fix: this commit copies the two compaction functions from the server code,
and modifies them to re-encrypt data when moved. This is not optimized at
all, if needed, we can improve this by a lot.

Also, for now only one execution path is handled from the two, as that's
the only one hit by sysbench. We'll have to figure out a testcase for
the other and fix that too, for now, it only contains an assert(0).
  • Loading branch information
dutow authored Sep 14, 2023
1 parent 3d128f9 commit e99d440
Show file tree
Hide file tree
Showing 8 changed files with 465 additions and 16 deletions.
450 changes: 447 additions & 3 deletions src/access/pg_tde_prune.c

Large diffs are not rendered by default.

2 changes: 0 additions & 2 deletions src/access/pg_tde_tdemap.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
*-------------------------------------------------------------------------
*/

#define TDE_FORK_DEBUG 1

#include "postgres.h"
#include "access/pg_tde_tdemap.h"
#include "transam/pg_tde_xact_handler.h"
Expand Down
4 changes: 3 additions & 1 deletion src/access/pg_tdeam.c
Original file line number Diff line number Diff line change
Expand Up @@ -8799,6 +8799,7 @@ pg_tde_xlog_prune(XLogReaderState *record)
int ndead;
int nunused;
Size datalen;
Relation reln;

redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);

Expand All @@ -8811,7 +8812,8 @@ pg_tde_xlog_prune(XLogReaderState *record)
Assert(nunused >= 0);

/* Update all line pointers per the record, and repair fragmentation */
pg_tde_page_prune_execute(buffer,
reln = CreateFakeRelcacheEntry(rlocator);
pg_tde_page_prune_execute(reln, buffer,
redirected, nredirected,
nowdead, ndead,
nowunused, nunused);
Expand Down
13 changes: 6 additions & 7 deletions src/encryption/enc_tuple.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#include "pg_tde_defines.h"
#define ENCRYPTION_DEBUG 1

#include "postgres.h"
#include "utils/memutils.h"
Expand All @@ -16,7 +15,7 @@

// t_data and out have to be different addresses without overlap!
// The only difference between enc and dec is how we calculate offsetInPage
static void PGTdeCryptTupInternal(Oid tableOid, BlockNumber bn, unsigned long offsetInPage, char* t_data, char* out, unsigned from, unsigned to)
void PGTdeCryptTupInternal(Oid tableOid, BlockNumber bn, unsigned long offsetInPage, char* t_data, char* out, unsigned from, unsigned to)
{
const uint64_t offsetInFile = (bn * BLCKSZ) + offsetInPage;

Expand Down Expand Up @@ -44,7 +43,7 @@ static void PGTdeCryptTupInternal(Oid tableOid, BlockNumber bn, unsigned long of
Aes128EncryptedZeroBlocks(ki->data.data, aesBlockNumber1, aesBlockNumber2, encKey);

#if ENCRYPTION_DEBUG
fprintf(stderr, " ---- (Oid: %i, Len: %u, AesBlock: %lu, BlockOffset: %lu) ----\n", tableOid, to - from, aesBlockNumber1, aesBlockOffset);
fprintf(stderr, " ---- (Oid: %i, Offset: %lu Len: %u, AesBlock: %lu, BlockOffset: %lu) ----\n", tableOid, offsetInPage, to - from, aesBlockNumber1, aesBlockOffset);
#endif
for(unsigned i = 0; i < to - from; ++i) {
const char v = ((char*)(t_data))[i + from];
Expand All @@ -56,7 +55,7 @@ static void PGTdeCryptTupInternal(Oid tableOid, BlockNumber bn, unsigned long of
}
}

static void PGTdeDecryptTupInternal(Oid tableOid, BlockNumber bn, Page page, HeapTupleHeader t_data, char* out, unsigned from, unsigned to)
void PGTdeDecryptTupInternal(Oid tableOid, BlockNumber bn, Page page, HeapTupleHeader t_data, char* out, unsigned from, unsigned to)
{
const unsigned long offsetInPage = (char*)t_data - (char*)page;
#if ENCRYPTION_DEBUG
Expand All @@ -66,7 +65,7 @@ static void PGTdeDecryptTupInternal(Oid tableOid, BlockNumber bn, Page page, Hea
}

// t_data and out have to be different addresses without overlap!
static void PGTdeEncryptTupInternal(Oid tableOid, BlockNumber bn, char* page, char* t_data, char* out, unsigned from, unsigned to)
void PGTdeEncryptTupInternal(Oid tableOid, BlockNumber bn, char* page, char* t_data, char* out, unsigned from, unsigned to)
{
const unsigned long offsetInPage = out - page;
#if ENCRYPTION_DEBUG
Expand Down Expand Up @@ -107,7 +106,7 @@ static void PGTdeDecryptTupInternal2(BlockNumber bn, Page page, HeapTuple tuple,

static void PGTdeDecryptTupData(BlockNumber bn, Page page, HeapTuple tuple)
{
PGTdeDecryptTupInternal2(bn, page, tuple, sizeof(HeapTupleHeaderData), tuple->t_len, true);
PGTdeDecryptTupInternal2(bn, page, tuple, tuple->t_data->t_hoff, tuple->t_len, true);
}

OffsetNumber
Expand All @@ -121,7 +120,7 @@ PGTdePageAddItemExtended(Oid oid,
{
OffsetNumber off = PageAddItemExtended(page,item,size,offsetNumber,flags);
PageHeader phdr = (PageHeader) page;
unsigned long headerSize = sizeof(HeapTupleHeaderData);
unsigned long headerSize = ((HeapTupleHeader)item)->t_hoff;

char* toAddr = ((char*)phdr) + phdr->pd_upper;

Expand Down
2 changes: 1 addition & 1 deletion src/include/access/pg_tdeam.h
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ extern int pg_tde_page_prune(Relation relation, Buffer buffer,
TimestampTz old_snap_ts,
int *nnewlpdead,
OffsetNumber *off_loc);
extern void pg_tde_page_prune_execute(Buffer buffer,
extern void pg_tde_page_prune_execute(Relation rel, Buffer buffer,
OffsetNumber *redirected, int nredirected,
OffsetNumber *nowdead, int ndead,
OffsetNumber *nowunused, int nunused);
Expand Down
4 changes: 4 additions & 0 deletions src/include/encryption/enc_tuple.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
#include "storage/bufpage.h"
#include "executor/tuptable.h"

void PGTdeCryptTupInternal(Oid tableOid, BlockNumber bn, unsigned long offsetInPage, char* t_data, char* out, unsigned from, unsigned to);
void PGTdeEncryptTupInternal(Oid tableOid, BlockNumber bn, char* page, char* t_data, char* out, unsigned from, unsigned to);
void PGTdeDecryptTupInternal(Oid tableOid, BlockNumber bn, Page page, HeapTupleHeader t_data, char* out, unsigned from, unsigned to);

/* A wrapper to encrypt a tuple before adding it to the buffer */
OffsetNumber
PGTdePageAddItemExtended(Oid oid, BlockNumber bn, Page page,
Expand Down
4 changes: 4 additions & 0 deletions src/include/pg_tde_defines.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
* ----------
*/

#define ENCRYPTION_DEBUG 1
#define KEYRING_DEBUG 1
#define TDE_FORK_DEBUG 1

#define pg_tde_fill_tuple heap_fill_tuple
#define pg_tde_form_tuple heap_form_tuple
#define pg_tde_deform_tuple heap_deform_tuple
Expand Down
2 changes: 0 additions & 2 deletions src/keyring/keyring_api.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@

#define KEYRING_DEBUG 1

#include "keyring/keyring_api.h"
#include "keyring/keyring_file.h"

Expand Down

0 comments on commit e99d440

Please sign in to comment.