From 31db70f1d460063ff84f1dafc8947c9660877d29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=97=B1=20PixelyIon?= Date: Thu, 18 Feb 2021 11:43:59 +0000 Subject: [PATCH] Lock decryption in CtrEncryptedBacking This prevents a race where two threads could read at the same time and end up using the wrong IV leading to garbage data being read. This caused crashes in several games including Celeste. --- app/src/main/cpp/skyline/crypto/aes_cipher.h | 1 + .../main/cpp/skyline/vfs/ctr_encrypted_backing.cpp | 14 ++++++++++---- .../main/cpp/skyline/vfs/ctr_encrypted_backing.h | 1 + 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/src/main/cpp/skyline/crypto/aes_cipher.h b/app/src/main/cpp/skyline/crypto/aes_cipher.h index 5ad1fdf9..b9d9c8f5 100644 --- a/app/src/main/cpp/skyline/crypto/aes_cipher.h +++ b/app/src/main/cpp/skyline/crypto/aes_cipher.h @@ -9,6 +9,7 @@ namespace skyline::crypto { /** * @brief Wrapper for mbedtls for AES decryption using a cipher + * @note The IV state must be appropriately locked during multi-threaded usage */ class AesCipher { private: diff --git a/app/src/main/cpp/skyline/vfs/ctr_encrypted_backing.cpp b/app/src/main/cpp/skyline/vfs/ctr_encrypted_backing.cpp index 37cdc6e8..75157e61 100644 --- a/app/src/main/cpp/skyline/vfs/ctr_encrypted_backing.cpp +++ b/app/src/main/cpp/skyline/vfs/ctr_encrypted_backing.cpp @@ -22,11 +22,14 @@ namespace skyline::vfs { size_t sectorOffset{offset % SectorSize}; if (sectorOffset == 0) { - UpdateCtr(baseOffset + offset); size_t read{backing->Read(output, offset)}; if (read != size) return 0; - cipher.Decrypt(output); + { + std::lock_guard guard(mutex); + UpdateCtr(baseOffset + offset); + cipher.Decrypt(output); + } return size; } @@ -35,8 +38,11 @@ namespace skyline::vfs { size_t read{backing->Read(blockBuf, sectorStart)}; if (read != SectorSize) return 0; - UpdateCtr(baseOffset + sectorStart); - cipher.Decrypt(blockBuf); + { + std::lock_guard guard(mutex); + UpdateCtr(baseOffset + sectorStart); + cipher.Decrypt(blockBuf); + } if (size + sectorOffset < SectorSize) { std::memcpy(output.data(), blockBuf.data() + sectorOffset, size); return size; diff --git a/app/src/main/cpp/skyline/vfs/ctr_encrypted_backing.h b/app/src/main/cpp/skyline/vfs/ctr_encrypted_backing.h index 6ed33c2a..b97d8e80 100644 --- a/app/src/main/cpp/skyline/vfs/ctr_encrypted_backing.h +++ b/app/src/main/cpp/skyline/vfs/ctr_encrypted_backing.h @@ -16,6 +16,7 @@ namespace skyline::vfs { crypto::KeyStore::Key128 ctr; crypto::AesCipher cipher; std::shared_ptr backing; + std::mutex mutex; //!< Synchronize all AES-CTR cipher state modifications size_t baseOffset; //!< The offset of the backing into the file is used to calculate the IV /**