tresor: discard snapshots only when writing the sb

Snapshots must only be removed when securing the superblock. Otherwise, the
last secured superblock might get corrupted. The Free Tree allocation algorithm
would not consider the deleted snapshots anymore although they are still active
in the secured superblock and re-use their blocks. This would render the tresor
container unusable if the superblock with the deleted snapshots is not secured
in the end (driver crash, power down, ...).

Ref #5077
This commit is contained in:
Martin Stein 2024-03-19 14:30:59 +01:00 committed by Christian Helmuth
parent 518c32e1af
commit 4a68f6bf75
2 changed files with 8 additions and 22 deletions

View File

@ -799,20 +799,6 @@ struct Tresor::Snapshots
snap.encode_to_blk(generator);
}
void discard_disposable_snapshots(Generation last_secured_gen,
Generation curr_gen)
{
for (Snapshot &snap : items) {
if (snap.valid &&
!snap.keep &&
snap.gen != curr_gen &&
snap.gen != last_secured_gen)
snap.valid = false;
}
}
Snapshot_index newest_snap_idx() const
{
Snapshot_index result { INVALID_SNAP_IDX };
@ -1039,6 +1025,13 @@ struct Tresor::Superblock
meta_degree = sb.meta_degree;
meta_leaves = sb.meta_leaves;
}
void discard_disposable_snapshots()
{
for (Snapshot &snap : snapshots.items)
if (snap.valid && snap.gen < last_secured_generation && !snap.keep)
snap.valid = false;
}
};

View File

@ -22,7 +22,6 @@ using namespace Tresor;
void Superblock_control::Write_vbas::_start_write_vba(Execute_attr const &attr, bool &progress)
{
Virtual_block_address vba = _attr.in_first_vba + _num_written_vbas;
attr.sb.snapshots.discard_disposable_snapshots(attr.sb.last_secured_generation, attr.curr_gen);
if (attr.sb.curr_snap().gen != attr.curr_gen) {
Snapshot &snap { attr.sb.curr_snap() };
attr.sb.curr_snap_idx = attr.sb.snapshots.alloc_idx(attr.curr_gen, attr.sb.last_secured_generation);
@ -123,7 +122,6 @@ bool Superblock_control::Extend_free_tree::execute(Execute_attr const &attr)
case INIT:
{
_num_pbas = _attr.in_num_pbas;
attr.sb.snapshots.discard_disposable_snapshots(attr.sb.last_secured_generation, attr.curr_gen);
Physical_block_address last_used_pba { attr.sb.first_pba + (attr.sb.nr_of_pbas - 1) };
Number_of_blocks nr_of_unused_pbas { MAX_PBA - last_used_pba };
@ -208,7 +206,6 @@ bool Superblock_control::Extend_vbd::execute(Execute_attr const &attr)
case INIT:
{
_num_pbas = _attr.in_num_pbas;
attr.sb.snapshots.discard_disposable_snapshots(attr.sb.last_secured_generation, attr.curr_gen);
Physical_block_address last_used_pba { attr.sb.first_pba + (attr.sb.nr_of_pbas - 1) };
Number_of_blocks nr_of_unused_pbas { MAX_PBA - last_used_pba };
@ -296,7 +293,6 @@ bool Superblock_control::Rekey::execute(Execute_attr const &attr)
switch (_helper.state) {
case INIT:
attr.sb.snapshots.discard_disposable_snapshots(attr.sb.last_secured_generation, attr.curr_gen);
_attr.out_rekeying_finished = false;
switch (attr.sb.state) {
case Superblock::NORMAL:
@ -390,6 +386,7 @@ bool Superblock_control::Secure_superblock::execute(Execute_attr const &attr)
case INIT:
attr.sb.curr_snap().gen = attr.curr_gen;
attr.sb.discard_disposable_snapshots();
_sb_ciphertext.copy_all_but_key_values_from(attr.sb);
_encrypt_key.generate(
_helper, ENCRYPT_KEY, ENCRYPT_CURR_KEY_SUCCEEDED, progress, _sb_ciphertext.current_key.value, attr.sb.current_key.value);
@ -453,7 +450,6 @@ bool Superblock_control::Discard_snapshot::execute(Execute_attr const &attr)
if (snap.valid && snap.gen == _attr.in_gen && snap.keep)
snap.keep = false;
attr.sb.snapshots.discard_disposable_snapshots(attr.sb.last_secured_generation, attr.curr_gen);
_secure_sb.generate(_helper, SECURE_SB, SECURE_SB_SUCCEEDED, progress);
break;
@ -477,7 +473,6 @@ bool Superblock_control::Create_snapshot::execute(Execute_attr const &attr)
_helper.mark_succeeded(progress);
} else {
snap.keep = true;
attr.sb.snapshots.discard_disposable_snapshots(attr.sb.last_secured_generation, attr.curr_gen);
_secure_sb.generate(_helper, SECURE_SB, SECURE_SB_SUCCEEDED, progress);
}
break;
@ -501,7 +496,6 @@ bool Superblock_control::Synchronize::execute(Execute_attr const &attr)
switch (_helper.state) {
case INIT:
attr.sb.snapshots.discard_disposable_snapshots(attr.sb.last_secured_generation, attr.curr_gen);
attr.sb.last_secured_generation = attr.curr_gen;
_secure_sb.generate(_helper, SECURE_SB, SECURE_SB_SUCCEEDED, progress);
break;
@ -576,7 +570,6 @@ bool Superblock_control::Deinitialize::execute(Execute_attr const &attr)
switch (_helper.state) {
case INIT:
attr.sb.snapshots.discard_disposable_snapshots(attr.sb.last_secured_generation, attr.curr_gen);
attr.sb.last_secured_generation = attr.curr_gen;
_secure_sb.generate(_helper, SECURE_SB, SECURE_SB_SUCCEEDED, progress);
break;