Fix more encryption issues on exFAT filesystem #7162
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t for new Realms leading to corruption for encrypted Realms
ironage
changed the title
Pull Request Test Coverage Report for Build james.stone_440
💛 - Coveralls |
…ave the same fallback tmp path which could cause issues if running on a filesystem that doesn't support mkfifo such as exFAT
|
Ping for reviews @finnschiermer @kiburtse @tgoyne |
tgoyne
reviewed
Nov 30, 2023
asserting in consideration of external processes. Use prealloc to size the file instead of a separate resize operation. Add the file path to help debug exceptions.
tgoyne
approved these changes
Dec 1, 2023
kiburtse
reviewed
Dec 1, 2023
kiburtse
approved these changes
Dec 5, 2023
Was encryption a feature of the 1.0.0 Realm Core release? |
|
@kraenhansen yes, there was encryption in v1.0.0 and I'm fairly certain the bug was present there because we had the static |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #7156
The issues here can be reproduced by running the core tests on an exFAT file system. See
tools/run-tests-on-exfat.shSpecifically, opening several new encrypted Realms at the same time in the same process. For example, I used
UNITTEST_FILTER=*_EncryptionKey*andUNITTEST_REPEAT=1000to reliably debug this.The issue is the same underlying issue as described in #6959 where the exFAT file system does not actually reserve the inode until the file is non-empty. The effect of having the unique id reused across multiple files in this case is that the static mappings_by_file structure merges encrypted mappings across different files. This can lead to data from one file ending up in the other, and general corruption.
In addition to fixing the issue, and cleaning up the surrounding code a bit, I have added an assertion in the
File::get_unique_id()method to check that it is called on a non-empty file so that we don't hit this bug again.☑️ ToDos
bindgen/spec.yml, if public C++ API changed