Fix several valgrind failures (#7789)

Author: tgoyne

CHANGELOG.md

@@ -6,7 +6,7 @@
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
-* None.
+* Valgrind could report a branch on an uninitialized read when opening something that is not an encrypted Realm file as an encrypted Realm file ([PR #7789](https://github.com/realm/realm-core/pull/7789), since v14.10.0).
 
 ### Breaking changes
 * None.

src/realm/util/encrypted_file_mapping.cpp

@@ -377,7 +377,7 @@ bool AESCryptor::constant_time_equals(const Hmac& a, const Hmac& b) const
 {
     // Constant-time memcmp to avoid timing attacks
     uint8_t result = 0;
-    for (size_t i = 0; i < 224 / 8; ++i)
+    for (size_t i = 0; i < a.size(); ++i)
         result |= a[i] ^ b[i];
     return result == 0;
 }
@@ -404,7 +404,7 @@ void AESCryptor::invalidate_ivs() noexcept
 AESCryptor::ReadResult AESCryptor::read(FileDesc fd, SizeType pos, char* dst, WriteObserver* observer)
 {
     uint32_t iv = 0;
-    Hmac hmac{};
+    Hmac hmac;
     // We're in a single-process scenario (or other processes are only reading),
     // so we can trust our in-memory caches and never need to retry
     if (!observer || observer->no_concurrent_writer_seen()) {
@@ -475,12 +475,12 @@ AESCryptor::ReadResult AESCryptor::attempt_read(FileDesc fd, SizeType pos, char*
     IVTable& iv = get_iv_table(fd, pos, iv_mode);
     iv_out = iv.iv1;
     if (iv.iv1 == 0) {
-        std::fill(hmac.begin(), hmac.end(), 0);
+        hmac.fill(0);
         return ReadResult::Uninitialized;
     }
 
     size_t actual = check_read(fd, data_pos_to_file_pos(pos), m_rw_buffer.get());
-    if (actual == 0) {
+    if (actual < encryption_page_size) {
         return ReadResult::Eof;
     }
 

test/test_group.cpp

@@ -119,7 +119,7 @@ TEST(Group_OpenUnencryptedFileWithKey_TwoPages)
         Group group;
         TableRef table = group.get_or_add_table("table");
         auto col = table->add_column(type_String, "str");
-        table->create_object().set(col, std::string(page_size() - 100, '\1'));
+        table->create_object().set(col, std::string(4096 - 100, '\1'));
         group.write(path);
     }
 
@@ -135,7 +135,7 @@ TEST(Group_OpenUnencryptedFileWithKey_ThreePages)
         Group group;
         TableRef table = group.get_or_add_table("table");
         auto col = table->add_column(type_String, "str");
-        std::string data(page_size() - 100, '\1');
+        std::string data(4096 - 100, '\1');
         table->create_object().set<String>(col, data);
         table->create_object().set<String>(col, data);
         group.write(path);
@@ -285,7 +285,7 @@ TEST(Group_TableNameTooLong)
 {
     Group group;
     size_t buf_len = 64;
-    std::unique_ptr<char[]> buf(new char[buf_len]);
+    std::unique_ptr<char[]> buf(new char[buf_len]());
     CHECK_LOGIC_ERROR(group.add_table(StringData(buf.get(), buf_len)), ErrorCodes::InvalidName);
     group.add_table(StringData(buf.get(), buf_len - 1));
 }

test/valgrind.suppress

@@ -1,52 +1,3 @@
-{
-   <AllocSlab::all_files and AllocSlab::all_files_mutex intentionally not destructed to prevent races>
-   Memcheck:Leak
-   match-leak-kinds: reachable
-   fun:_Znwm
-   fun:_GLOBAL__sub_I_alloc_slab.cpp
-   fun:call_init.part.0
-   fun:call_init
-   fun:_dl_init
-   ...
-}
-{
-   <Entries added to the AllocSlab::all_files map are not freed namely `p = std::make_shared<MappedFile>();`>
-   Memcheck:Leak
-   match-leak-kinds: reachable
-   fun:_Znwm
-   fun:_ZNSt8_Rb_treeISsSt4pairIKSsSt8weak_ptrIN5realm9SlabAlloc10MappedFileEEESt10_Select1stIS7_ESt4lessISsESaIS7_EE22_M_emplace_hint_uniqueIIRKSt21piecewise_construct_tSt5tupleIIRS1_EESI_IIEEEEESt17_Rb_tree_iteratorIS7_ESt23_Rb_tree_const_iteratorIS7_EDpOT_
-   fun:_ZN5realm9SlabAlloc11attach_fileERKSsRNS0_6ConfigE
-   fun:_ZN5realm5Group4openERKSsPKcNS0_8OpenModeE
-   ...
-}
-{
-   <std weak pointer allocated for entries in AllocSlab::all_files as above>
-   Memcheck:Leak
-   match-leak-kinds: reachable
-   fun:_Znwm
-   fun:_ZNSt8_Rb_treeISsSt4pairIKSsSt8weak_ptrIN5realm9SlabAlloc10MappedFileEEESt10_Select1stIS7_ESt4lessISsESaIS7_EE22_M_emplace_hint_uniqueIIRKSt21piecewise_construct_tSt5tupleIIRS1_EESI_IIEEEEESt17_Rb_tree_iteratorIS7_ESt23_Rb_tree_const_iteratorIS7_EDpOT_
-   fun:_ZN5realm9SlabAlloc11attach_fileERKSsRNS0_6ConfigE
-   fun:_ZN5realm11SharedGroup7do_openERKSsbbNS_18SharedGroupOptionsE
-   fun:_ZN5realm11SharedGroupC2ERKSsbNS_18SharedGroupOptionsE.constprop.432
-   ...
-}
-{
-   <We construct shared pointers with new. This covers two instances: `m_local_mappings.reset(new ...` and `new_mappings.reset(new ...`>
-   Memcheck:Leak
-   match-leak-kinds: reachable
-   fun:_Znwm
-   fun:_ZN5realm9SlabAlloc11attach_fileERKSsRNS0_6ConfigE
-   ...
-}
-{
-   <SlabAlloc makes a deep copy of the realm file_path string which is still reachable as the keys of the all_files map>
-   Memcheck:Leak
-   fun:_Znwm
-   fun:_ZNSs4_Rep9_S_createEmmRKSaIcE
-   ...
-   fun:_ZN5realm9SlabAlloc11attach_fileERKSsRNS0_6ConfigE
-   ...
-}
 {
    <Supposed valgrind false positives>
    Memcheck:Leak