From 70a7bca92014b1698c3bfc80e7d61725b9b3222f Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Sat, 26 Nov 2022 01:17:43 +0100 Subject: LibSQL: Fix BTree corruption in `TreeNode::split` After splitting a node, the new node was written to the same pointer as the current node - probably a copy / paste error. This new code requires a `.pointer() -> u32` to exist on the object to be serialized, preventing this issue from happening again. Fixes #15844. --- Userland/Libraries/LibSQL/BTree.cpp | 2 +- Userland/Libraries/LibSQL/BTreeIterator.cpp | 2 +- Userland/Libraries/LibSQL/Database.cpp | 2 +- Userland/Libraries/LibSQL/HashIndex.cpp | 12 ++++++------ Userland/Libraries/LibSQL/Serializer.h | 4 ++-- Userland/Libraries/LibSQL/TreeNode.cpp | 10 +++++----- 6 files changed, 16 insertions(+), 16 deletions(-) (limited to 'Userland') diff --git a/Userland/Libraries/LibSQL/BTree.cpp b/Userland/Libraries/LibSQL/BTree.cpp index d83a353e8d..179af103b3 100644 --- a/Userland/Libraries/LibSQL/BTree.cpp +++ b/Userland/Libraries/LibSQL/BTree.cpp @@ -56,7 +56,7 @@ TreeNode* BTree::new_root() { set_pointer(new_record_pointer()); m_root = make(*this, nullptr, m_root.leak_ptr(), pointer()); - serializer().serialize_and_write(*m_root.ptr(), m_root->pointer()); + serializer().serialize_and_write(*m_root.ptr()); if (on_new_root) on_new_root(); return m_root; diff --git a/Userland/Libraries/LibSQL/BTreeIterator.cpp b/Userland/Libraries/LibSQL/BTreeIterator.cpp index d062287489..f761c39bee 100644 --- a/Userland/Libraries/LibSQL/BTreeIterator.cpp +++ b/Userland/Libraries/LibSQL/BTreeIterator.cpp @@ -232,7 +232,7 @@ bool BTreeIterator::update(Key const& new_value) // We are friend of BTree and TreeNode. Don't know how I feel about that. m_current->m_entries[m_index] = new_value; - m_current->tree().serializer().serialize_and_write(*m_current, m_current->pointer()); + m_current->tree().serializer().serialize_and_write(*m_current); return true; } diff --git a/Userland/Libraries/LibSQL/Database.cpp b/Userland/Libraries/LibSQL/Database.cpp index 3f25267632..55a8af7dac 100644 --- a/Userland/Libraries/LibSQL/Database.cpp +++ b/Userland/Libraries/LibSQL/Database.cpp @@ -223,7 +223,7 @@ ErrorOr Database::update(Row& tuple) VERIFY(m_table_cache.get(tuple.table()->key().hash()).has_value()); // TODO Check constraints m_serializer.reset(); - m_serializer.serialize_and_write(tuple, tuple.pointer()); + m_serializer.serialize_and_write(tuple); // TODO update indexes defined on table. return {}; diff --git a/Userland/Libraries/LibSQL/HashIndex.cpp b/Userland/Libraries/LibSQL/HashIndex.cpp index 2642ada69a..bde86fbb7d 100644 --- a/Userland/Libraries/LibSQL/HashIndex.cpp +++ b/Userland/Libraries/LibSQL/HashIndex.cpp @@ -140,7 +140,7 @@ bool HashBucket::insert(Key const& key) return false; } m_entries.append(key); - m_hash_index.serializer().serialize_and_write(*this, pointer()); + m_hash_index.serializer().serialize_and_write(*this); return true; } @@ -226,10 +226,10 @@ HashIndex::HashIndex(Serializer& serializer, NonnullRefPtr cons } else { auto bucket = append_bucket(0u, 1u, new_record_pointer()); bucket->m_inflated = true; - serializer.serialize_and_write(*bucket, bucket->pointer()); + serializer.serialize_and_write(*bucket); bucket = append_bucket(1u, 1u, new_record_pointer()); bucket->m_inflated = true; - serializer.serialize_and_write(*bucket, bucket->pointer()); + serializer.serialize_and_write(*bucket); m_nodes.append(first_node); write_directory_to_write_ahead_log(); } @@ -283,7 +283,7 @@ HashBucket* HashIndex::get_bucket_for_insert(Key const& key) } if (moved > 0) { dbgln_if(SQL_DEBUG, "Moved {} entries from bucket #{} to #{}", moved, base_index, ix); - serializer().serialize_and_write(*sub_bucket, sub_bucket->pointer()); + serializer().serialize_and_write(*sub_bucket); } total_moved += moved; } @@ -292,7 +292,7 @@ HashBucket* HashIndex::get_bucket_for_insert(Key const& key) else dbgln_if(SQL_DEBUG, "Nothing redistributed from bucket #{}", base_index); bucket->set_local_depth(bucket->local_depth() + 1); - serializer().serialize_and_write(*bucket, bucket->pointer()); + serializer().serialize_and_write(*bucket); write_directory_to_write_ahead_log(); auto bucket_after_redistribution = get_bucket(key_hash % size()); @@ -327,7 +327,7 @@ void HashIndex::write_directory_to_write_ahead_log() size_t num_node = 0u; while (offset < size()) { HashDirectoryNode node(*this, num_node, offset); - serializer().serialize_and_write(node, node.pointer()); + serializer().serialize_and_write(node); offset += node.number_of_pointers(); } } diff --git a/Userland/Libraries/LibSQL/Serializer.h b/Userland/Libraries/LibSQL/Serializer.h index 914a99fa0f..0cb089be67 100644 --- a/Userland/Libraries/LibSQL/Serializer.h +++ b/Userland/Libraries/LibSQL/Serializer.h @@ -108,12 +108,12 @@ public: void serialize(String const&); template - bool serialize_and_write(T const& t, u32 pointer) + bool serialize_and_write(T const& t) { VERIFY(m_heap.ptr() != nullptr); reset(); serialize(t); - m_heap->add_to_wal(pointer, m_buffer); + m_heap->add_to_wal(t.pointer(), m_buffer); return true; } diff --git a/Userland/Libraries/LibSQL/TreeNode.cpp b/Userland/Libraries/LibSQL/TreeNode.cpp index 749570e2cb..17759d0df5 100644 --- a/Userland/Libraries/LibSQL/TreeNode.cpp +++ b/Userland/Libraries/LibSQL/TreeNode.cpp @@ -172,7 +172,7 @@ bool TreeNode::update_key_pointer(Key const& key) if (m_entries[ix].pointer() != key.pointer()) { m_entries[ix].set_pointer(key.pointer()); dump_if(SQL_DEBUG, "To WAL"); - tree().serializer().serialize_and_write(*this, pointer()); + tree().serializer().serialize_and_write(*this); } return true; } @@ -277,7 +277,7 @@ void TreeNode::just_insert(Key const& key, TreeNode* right) split(); } else { dump_if(SQL_DEBUG, "To WAL"); - tree().serializer().serialize_and_write(*this, pointer()); + tree().serializer().serialize_and_write(*this); } return; } @@ -289,7 +289,7 @@ void TreeNode::just_insert(Key const& key, TreeNode* right) split(); } else { dump_if(SQL_DEBUG, "To WAL"); - tree().serializer().serialize_and_write(*this, pointer()); + tree().serializer().serialize_and_write(*this); } } @@ -327,9 +327,9 @@ void TreeNode::split() auto median = m_entries.take_last(); dump_if(SQL_DEBUG, "Split Left To WAL"); - tree().serializer().serialize_and_write(*this, pointer()); + tree().serializer().serialize_and_write(*this); new_node->dump_if(SQL_DEBUG, "Split Right to WAL"); - tree().serializer().serialize_and_write(*new_node, pointer()); + tree().serializer().serialize_and_write(*new_node); m_up->just_insert(median, new_node); } -- cgit v1.2.3