Clean up a bunch of old encryption cruft

The global shared cache of encrypted file maps was originally required because
we actually opened Realm files mulitple times in normal usage, so each of the
open files had to know about each other to copy things around. #4839 made it so
that in normal usage we only ever have one DB instance per file per process, so
it became dead code. Multiprocess encryption made it unneccesary even when the
one-DB-per-process rule is violated, as the multiprocess code path covers that.

This eliminates our last reliance on file UniqueIDs, so it lets us get rid of
hacks related to that.

The encryption page reclaimer mostly never actually worked. It used a very
conserative page reclaimation rule that meant that pages would never be
reclaimed if there was a long-lived Transaction, even if it was frozen or kept
refreshed. This is very common in practice, and when it doesn't happen the DB
usually isn't kept open either, making it redundant.

Encryption used to rely on handling BAD_EXEC signals (or mach exceptions)
rather than explicit barriers, so it had to read and write in page-sized
chunks. That's no longer the case, so we can eliminate a lot of complexity by
always reading and writing in 4k blocks.

Author: tgoyne

CHANGELOG.md

@@ -8,6 +8,10 @@
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
 * Fix some client resets (such as migrating to flexible sync) potentially failing with AutoClientResetFailed if a new client reset condition (such as rolling back a flexible sync migration) occurred before the first one completed. ([PR #7542](https://github.com/realm/realm-core/pull/7542), since v13.11.0)
+* Encrypted files on Windows had a maximum size of 2GB even on x64 due to internal usage of `off_t`, which is a 32-bit type on 64-bit Windows ([PR #7698](https://github.com/realm/realm-core/pull/7698), since the introduction of encryption support on Windows in v3.0.0).
+* The encryption code no longer behaves differently depending on the system page size, which should entirely eliminate a recurring source of bugs related to copying encrypted Realm files between platforms with different page sizes. One known outstanding bug was ([RNET-1141](https://github.com/realm/realm-dotnet/issues/3592)), where opening files on a system with a larger page size than the writing system would attempt to read sections of the file which had never been written to ([PR #7698](https://github.com/realm/realm-core/pull/7698)).
+* There were several complicated scenarios which could result in stale reads from encrypted files in multiprocess scenarios. These were very difficult to hit and would typically lead to a crash, either due to an assertion failure or DecryptionFailure being thrown ([PR #7698](https://github.com/realm/realm-core/pull/7698), since v13.9.0).
+* Encrypted files have some benign data races where we can memcpy a block of memory while another thread is writing to a limited range of it. It is logically impossible to ever read from that range when this happens, but Thread Sanitizer quite reasonably complains about this. We now perform a slower operations when running with TSan which avoids this benign race ([PR #7698](https://github.com/realm/realm-core/pull/7698)).
 
 ### Breaking changes
 * Any `stitch_` prefixed fields in the `BsonDocument` returned from `app::User::custom_data()` are being renamed on the server to have a `baas_` prefix instead ([PR #7769](https://github.com/realm/realm-core/pull/7769)).
@@ -21,6 +25,7 @@
 * Removed references to `stitch_` fields in access tokens in sync unit tests ([PR #7769](https://github.com/realm/realm-core/pull/7769)).
 * Added back iOS simulator testing to evergreen after Jenkins went away ([PR #7758](https://github.com/realm/realm-core/pull/7758)).
 * `realm-trawler -c` did not work on Realm using SyncClient history ([PR #7734](https://github.com/realm/realm-core/pull/7734)).
+* `File::Map`'s move constructor and assignment operator left `m_fd` unchanged, which appears to have never actually resulted in problems with how it was used ([PR #7698](https://github.com/realm/realm-core/pull/7698)).
 
 ----------------------------------------------
 

src/realm/alloc.cpp

@@ -119,9 +119,7 @@ char* Allocator::translate_less_critical(RefTranslation* ref_translation_ptr, re
     RefTranslation& txl = ref_translation_ptr[idx];
     size_t offset = ref - get_section_base(idx);
     char* addr = txl.mapping_addr + offset;
-#if REALM_ENABLE_ENCRYPTION
-    realm::util::encryption_read_barrier(addr, NodeHeader::header_size, txl.encrypted_mapping, nullptr);
-#endif
+    util::encryption_read_barrier(addr, NodeHeader::header_size, txl.encrypted_mapping);
     auto size = NodeHeader::get_byte_size_from_header(addr);
     bool crosses_mapping = offset + size > (1 << section_shift);
     // Move the limit on use of the existing primary mapping.
@@ -135,27 +133,21 @@ char* Allocator::translate_less_critical(RefTranslation* ref_translation_ptr, re
     }
     if (REALM_LIKELY(!crosses_mapping)) {
         // Array fits inside primary mapping, no new mapping needed.
-#if REALM_ENABLE_ENCRYPTION
-        realm::util::encryption_read_barrier(addr, size, txl.encrypted_mapping, nullptr);
-#endif
+        util::encryption_read_barrier(addr, size, txl.encrypted_mapping);
         return addr;
     }
-    else {
-        // we need a cross-over mapping. If one is already established, use that.
-        auto xover_mapping_addr = txl.xover_mapping_addr.load(std::memory_order_acquire);
-        if (!xover_mapping_addr) {
-            // we need to establish a xover mapping - or wait for another thread to finish
-            // establishing one:
-            const_cast<Allocator*>(this)->get_or_add_xover_mapping(txl, idx, offset, size);
-            // reload (can be relaxed since the call above synchronizes on a mutex)
-            xover_mapping_addr = txl.xover_mapping_addr.load(std::memory_order_relaxed);
-        }
-        // array is now known to be inside the established xover mapping:
-        addr = xover_mapping_addr + (offset - txl.xover_mapping_base);
-#if REALM_ENABLE_ENCRYPTION
-        realm::util::encryption_read_barrier(addr, size, txl.xover_encrypted_mapping, nullptr);
-#endif
-        return addr;
+    // we need a cross-over mapping. If one is already established, use that.
+    auto xover_mapping_addr = txl.xover_mapping_addr.load(std::memory_order_acquire);
+    if (!xover_mapping_addr) {
+        // we need to establish a xover mapping - or wait for another thread to finish
+        // establishing one:
+        const_cast<Allocator*>(this)->get_or_add_xover_mapping(txl, idx, offset, size);
+        // reload (can be relaxed since the call above synchronizes on a mutex)
+        xover_mapping_addr = txl.xover_mapping_addr.load(std::memory_order_relaxed);
     }
+    // array is now known to be inside the established xover mapping:
+    addr = xover_mapping_addr + (offset - txl.xover_mapping_base);
+    util::encryption_read_barrier(addr, size, txl.xover_encrypted_mapping);
+    return addr;
 }
 } // namespace realm

src/realm/alloc.hpp

@@ -171,7 +171,7 @@ class Allocator {
     // into equal chunks.
     struct RefTranslation {
         char* mapping_addr;
-        uint64_t cookie;
+        uint64_t cookie = 0x1234567890;
         std::atomic<size_t> lowest_possible_xover_offset = 0;
 
         // member 'xover_mapping_addr' is used for memory synchronization of the fields
@@ -183,14 +183,12 @@ class Allocator {
 #if REALM_ENABLE_ENCRYPTION
         util::EncryptedFileMapping* encrypted_mapping = nullptr;
         util::EncryptedFileMapping* xover_encrypted_mapping = nullptr;
+#else
+        static inline util::EncryptedFileMapping* const encrypted_mapping = nullptr;
+        static inline util::EncryptedFileMapping* const xover_encrypted_mapping = nullptr;
 #endif
-        explicit RefTranslation(char* addr)
+        explicit RefTranslation(char* addr = nullptr)
             : mapping_addr(addr)
-            , cookie(0x1234567890)
-        {
-        }
-        RefTranslation()
-            : RefTranslation(nullptr)
         {
         }
         ~RefTranslation()
@@ -222,7 +220,7 @@ class Allocator {
     };
     // This pointer may be changed concurrently with access, so make sure it is
     // atomic!
-    std::atomic<RefTranslation*> m_ref_translation_ptr;
+    std::atomic<RefTranslation*> m_ref_translation_ptr{nullptr};
 
     /// The specified size must be divisible by 8, and must not be
     /// zero.
@@ -252,7 +250,7 @@ class Allocator {
     char* translate_critical(RefTranslation*, ref_type ref) const noexcept;
     char* translate_less_critical(RefTranslation*, ref_type ref) const noexcept;
     virtual void get_or_add_xover_mapping(RefTranslation&, size_t, size_t, size_t) = 0;
-    Allocator() noexcept;
+    Allocator() noexcept = default;
     size_t get_section_index(size_t pos) const noexcept;
     inline size_t get_section_base(size_t index) const noexcept;
 
@@ -271,11 +269,9 @@ class Allocator {
     //   used to detect if the allocator (and owning structure, e.g. Table)
     //   is recycled. Mismatch on this counter will cause accesors
     //   lower in the hierarchy to throw if access is attempted.
-    std::atomic<uint_fast64_t> m_content_versioning_counter;
-
-    std::atomic<uint_fast64_t> m_storage_versioning_counter;
-
-    std::atomic<uint_fast64_t> m_instance_versioning_counter;
+    std::atomic<uint_fast64_t> m_content_versioning_counter{0};
+    std::atomic<uint_fast64_t> m_storage_versioning_counter{0};
+    std::atomic<uint_fast64_t> m_instance_versioning_counter{0};
 
     inline uint_fast64_t get_storage_version(uint64_t instance_version)
     {
@@ -547,14 +543,6 @@ inline bool Allocator::is_read_only(ref_type ref) const noexcept
     return ref < m_baseline.load(std::memory_order_relaxed);
 }
 
-inline Allocator::Allocator() noexcept
-{
-    m_content_versioning_counter = 0;
-    m_storage_versioning_counter = 0;
-    m_instance_versioning_counter = 0;
-    m_ref_translation_ptr = nullptr;
-}
-
 // performance critical part of the translation process. Less critical code is in translate_less_critical.
 inline char* Allocator::translate_critical(RefTranslation* ref_translation_ptr, ref_type ref) const noexcept
 {
@@ -566,30 +554,23 @@ inline char* Allocator::translate_critical(RefTranslation* ref_translation_ptr,
         if (REALM_LIKELY(offset < lowest_possible_xover_offset)) {
             // the lowest possible xover offset may grow concurrently, but that will not affect this code path
             char* addr = txl.mapping_addr + offset;
-#if REALM_ENABLE_ENCRYPTION
-            realm::util::encryption_read_barrier(addr, NodeHeader::header_size, txl.encrypted_mapping,
-                                                 NodeHeader::get_byte_size_from_header);
-#endif
+            util::encryption_read_barrier(addr, NodeHeader::header_size, txl.encrypted_mapping);
+            size_t size = NodeHeader::get_byte_size_from_header(addr);
+            util::encryption_read_barrier(addr, size, txl.encrypted_mapping);
             return addr;
         }
-        else {
-            // the lowest possible xover offset may grow concurrently, but that will be handled inside the call
-            return translate_less_critical(ref_translation_ptr, ref);
-        }
+        // the lowest possible xover offset may grow concurrently, but that will be handled inside the call
+        return translate_less_critical(ref_translation_ptr, ref);
     }
     realm::util::terminate("Invalid ref translation entry", __FILE__, __LINE__, txl.cookie, 0x1234567890, ref, idx);
-    return nullptr;
 }
 
 inline char* Allocator::translate(ref_type ref) const noexcept
 {
-    auto ref_translation_ptr = m_ref_translation_ptr.load(std::memory_order_acquire);
-    if (REALM_LIKELY(ref_translation_ptr)) {
-        return translate_critical(ref_translation_ptr, ref);
-    }
-    else {
-        return do_translate(ref);
+    if (auto ptr = m_ref_translation_ptr.load(std::memory_order_acquire); REALM_LIKELY(ptr)) {
+        return translate_critical(ptr, ref);
     }
+    return do_translate(ref);
 }