From 3f888d27455aa10afd0f664849220eed46fb15e0 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Mon, 20 Jul 2020 13:04:45 -0700 Subject: [PATCH] Fix reattaching the allocator after compact SlabAlloc::detach() sets m_youngest_live_version to 0 and attach() doesn't touch it (as it comes from the lockfile rather than anything it has access to), so compact needs to set m_youngest_live_version after attaching the allocator to the new Realm file. Failing to do this resulted in the first ref translation table being deallocated after the first write transaction even if it was still in use, resulting in a use-after-free for whatever was using it. --- CHANGELOG.md | 1 + src/realm/db.cpp | 1 + test/test_shared.cpp | 25 +++++++++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbb864af7e4..b3074f9c23a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) * Fix upgrade bug. Could cause assertions like "Assertion failed: ref != 0" during opning of a realm. ([#6644](https://github.com/realm/realm-cocoa/issues/6644), since V6.0.7) +* A use-after-free would occur if a Realm was compacted, opened on multiple threads prior to the first write, then written to while reads were happening on other threads. This could result in a variety of crashes, often inside realm::util::EncryptedFileMapping::read_barrier. (Since v6.0.0, [Cocoa #6652](https://github.com/realm/realm-cocoa/issues/6652), [Cocoa #6628](https://github.com/realm/realm-cocoa/issues/6628), [Cocoa #6655](https://github.com/realm/realm-cocoa/issues/6555)). ### Breaking changes * None. diff --git a/src/realm/db.cpp b/src/realm/db.cpp index 1dd82f5c0af..ae692a210c4 100644 --- a/src/realm/db.cpp +++ b/src/realm/db.cpp @@ -1453,6 +1453,7 @@ bool DB::compact(bool bump_version_number, util::Optional output_en cfg.encryption_key = write_key; ref_type top_ref; top_ref = m_alloc.attach_file(m_db_path, cfg); + m_alloc.m_youngest_live_version = info->latest_version_number; info->number_of_versions = 1; // info->latest_version_number is unchanged SharedInfo* r_info = m_reader_map.get_addr(); diff --git a/test/test_shared.cpp b/test/test_shared.cpp index 65a582d484a..a05a1e44b9d 100644 --- a/test/test_shared.cpp +++ b/test/test_shared.cpp @@ -499,6 +499,31 @@ TEST(Shared_CompactingOnTheFly) } +TEST(Shared_ReadAfterCompact) +{ + SHARED_GROUP_TEST_PATH(path); + DBRef sg = DB::create(path); + { + WriteTransaction wt(sg); + auto table = wt.add_table("table"); + table->add_column(type_Int, "col"); + table->create_object().set_all(1); + wt.commit(); + } + sg->compact(); + auto rt = sg->start_read(); + auto table = rt->get_table("table"); + for (int i = 2; i < 4; ++i) { + WriteTransaction wt(sg); + wt.get_table("table")->create_object().set_all(i); + wt.commit(); + } + + CHECK_EQUAL(table->size(), 1); + CHECK_EQUAL(table->get_object(0).get("col"), 1); +} + + TEST(Shared_EncryptedRemap) { // Attempts to trigger code coverage in util::mremap() for the case where the file is encrypted.