diff --git a/src/Sentry.Android.AssemblyReader/V2/AssemblyStoreReader.cs b/src/Sentry.Android.AssemblyReader/V2/AssemblyStoreReader.cs index a3201754e1..2cfb0717ab 100644 --- a/src/Sentry.Android.AssemblyReader/V2/AssemblyStoreReader.cs +++ b/src/Sentry.Android.AssemblyReader/V2/AssemblyStoreReader.cs @@ -11,6 +11,7 @@ internal abstract class AssemblyStoreReader private static readonly UTF8Encoding ReaderEncoding = new UTF8Encoding(false); + internal object StreamLock { get; } = new(); protected Stream StoreStream { get; } public abstract string Description { get; } @@ -44,34 +45,37 @@ protected AssemblyStoreReader(Stream store, string path, DebugLogger? logger) protected BinaryReader CreateReader() => new BinaryReader(StoreStream, ReaderEncoding, leaveOpen: true); - protected abstract bool IsSupported(); + protected internal abstract bool IsSupported(); protected abstract void Prepare(); protected abstract ulong GetStoreStartDataOffset(); public MemoryStream ReadEntryImageData(AssemblyStoreItem entry, bool uncompressIfNeeded = false) { - ulong startOffset = GetStoreStartDataOffset(); - StoreStream.Seek((uint)startOffset + entry.DataOffset, SeekOrigin.Begin); - var stream = new MemoryStream(); - - if (uncompressIfNeeded) - { - throw new NotImplementedException(); - } - - const long BufferSize = 65535; - byte[] buffer = Utils.BytePool.Rent((int)BufferSize); - long remainingToRead = entry.DataSize; - - while (remainingToRead > 0) + lock (StreamLock) { - int nread = StoreStream.Read(buffer, 0, (int)Math.Min(BufferSize, remainingToRead)); - stream.Write(buffer, 0, nread); - remainingToRead -= (long)nread; + ulong startOffset = GetStoreStartDataOffset(); + StoreStream.Seek((uint)startOffset + entry.DataOffset, SeekOrigin.Begin); + var stream = new MemoryStream(); + + if (uncompressIfNeeded) + { + throw new NotImplementedException(); + } + + const long BufferSize = 65535; + byte[] buffer = Utils.BytePool.Rent((int)BufferSize); + long remainingToRead = entry.DataSize; + + while (remainingToRead > 0) + { + int nread = StoreStream.Read(buffer, 0, (int)Math.Min(BufferSize, remainingToRead)); + stream.Write(buffer, 0, nread); + remainingToRead -= (long)nread; + } + stream.Flush(); + stream.Seek(0, SeekOrigin.Begin); + + return stream; } - stream.Flush(); - stream.Seek(0, SeekOrigin.Begin); - - return stream; } } diff --git a/src/Sentry.Android.AssemblyReader/V2/StoreReader.Classes.cs b/src/Sentry.Android.AssemblyReader/V2/StoreReader.Classes.cs index 5ed713b6be..2776a21081 100644 --- a/src/Sentry.Android.AssemblyReader/V2/StoreReader.Classes.cs +++ b/src/Sentry.Android.AssemblyReader/V2/StoreReader.Classes.cs @@ -31,7 +31,7 @@ public Header(uint magic, uint version, uint entry_count, uint index_entry_count } } - private sealed class IndexEntry + internal sealed class IndexEntry { public readonly ulong name_hash; public readonly uint descriptor_index; @@ -45,7 +45,7 @@ public IndexEntry(ulong name_hash, uint descriptor_index, bool ignore) } } - private sealed class EntryDescriptor + internal sealed class EntryDescriptor { public uint mapping_index; @@ -59,7 +59,7 @@ private sealed class EntryDescriptor public uint config_data_size; } - private sealed class StoreItemV2 : AssemblyStoreItem + internal sealed class StoreItemV2 : AssemblyStoreItem { public StoreItemV2(AndroidTargetArch targetArch, string name, bool is64Bit, List indexEntries, EntryDescriptor descriptor, bool ignore) diff --git a/src/Sentry.Android.AssemblyReader/V2/StoreReader.cs b/src/Sentry.Android.AssemblyReader/V2/StoreReader.cs index 5220bf7dc7..d93b714703 100644 --- a/src/Sentry.Android.AssemblyReader/V2/StoreReader.cs +++ b/src/Sentry.Android.AssemblyReader/V2/StoreReader.cs @@ -91,163 +91,163 @@ public StoreReader(Stream store, string path, DebugLogger? logger) protected override ulong GetStoreStartDataOffset() => elfOffset; - protected override bool IsSupported() + protected internal override bool IsSupported() { - StoreStream.Seek(0, SeekOrigin.Begin); - using var reader = CreateReader(); - - uint magic = reader.ReadUInt32(); - if (magic == Utils.ELFMagic) + lock (StreamLock) { - ELFPayloadError error; - (elfOffset, _, error) = Utils.FindELFPayloadSectionOffsetAndSize(StoreStream); + StoreStream.Seek(0, SeekOrigin.Begin); + using var reader = CreateReader(); - if (error != ELFPayloadError.None) + var magic = reader.ReadUInt32(); + if (magic == Utils.ELFMagic) { - string message = error switch + (elfOffset, _, var error) = Utils.FindELFPayloadSectionOffsetAndSize(StoreStream); + + if (error != ELFPayloadError.None) { - ELFPayloadError.NotELF => $"Store '{StorePath}' is not a valid ELF binary", - ELFPayloadError.LoadFailed => $"Store '{StorePath}' could not be loaded", - ELFPayloadError.NotSharedLibrary => $"Store '{StorePath}' is not a shared ELF library", - ELFPayloadError.NotLittleEndian => $"Store '{StorePath}' is not a little-endian ELF image", - ELFPayloadError.NoPayloadSection => $"Store '{StorePath}' does not contain the 'payload' section", - _ => $"Unknown ELF payload section error for store '{StorePath}': {error}" - }; - Logger?.Invoke(DebugLoggerLevel.Debug, message); - // Was originally: - // ``` - // } else if (elfOffset >= 0) { - // ``` - // However since elfOffset is an ulong, it will never be less than 0 + var message = error switch + { + ELFPayloadError.NotELF => $"Store '{StorePath}' is not a valid ELF binary", + ELFPayloadError.LoadFailed => $"Store '{StorePath}' could not be loaded", + ELFPayloadError.NotSharedLibrary => $"Store '{StorePath}' is not a shared ELF library", + ELFPayloadError.NotLittleEndian => $"Store '{StorePath}' is not a little-endian ELF image", + ELFPayloadError.NoPayloadSection => $"Store '{StorePath}' does not contain the 'payload' section", + _ => $"Unknown ELF payload section error for store '{StorePath}': {error}" + }; + Logger?.Invoke(DebugLoggerLevel.Debug, message); + } + else + { + StoreStream.Seek((long)elfOffset, SeekOrigin.Begin); + magic = reader.ReadUInt32(); + } } - else + + if (magic != Utils.AssemblyStoreMagic) { - StoreStream.Seek((long)elfOffset, SeekOrigin.Begin); - magic = reader.ReadUInt32(); + Logger?.Invoke(DebugLoggerLevel.Debug, "Store '{0}' has invalid header magic number.", StorePath); + return false; } - } - if (magic != Utils.AssemblyStoreMagic) - { - Logger?.Invoke(DebugLoggerLevel.Debug, "Store '{0}' has invalid header magic number.", StorePath); - return false; - } - - uint version = reader.ReadUInt32(); - if (!supportedVersions.Contains(version)) - { - Logger?.Invoke(DebugLoggerLevel.Debug, "Store '{0}' has unsupported version 0x{1:x}", StorePath, version); - return false; - } + var version = reader.ReadUInt32(); + if (!supportedVersions.Contains(version)) + { + Logger?.Invoke(DebugLoggerLevel.Debug, "Store '{0}' has unsupported version 0x{1:x}", StorePath, version); + return false; + } - uint entry_count = reader.ReadUInt32(); - uint index_entry_count = reader.ReadUInt32(); - uint index_size = reader.ReadUInt32(); + var entry_count = reader.ReadUInt32(); + var index_entry_count = reader.ReadUInt32(); + var index_size = reader.ReadUInt32(); - header = new Header(magic, version, entry_count, index_entry_count, index_size); - return true; + header = new Header(magic, version, entry_count, index_entry_count, index_size); + return true; + } } protected override void Prepare() { - if (header == null) + lock (StreamLock) { - throw new InvalidOperationException("Internal error: header not set, was IsSupported() called?"); - } + if (header == null) + { + throw new InvalidOperationException("Internal error: header not set, was IsSupported() called?"); + } - TargetArch = (header.version & ASSEMBLY_STORE_ABI_MASK) switch - { - ASSEMBLY_STORE_ABI_AARCH64 => AndroidTargetArch.Arm64, - ASSEMBLY_STORE_ABI_ARM => AndroidTargetArch.Arm, - ASSEMBLY_STORE_ABI_X64 => AndroidTargetArch.X86_64, - ASSEMBLY_STORE_ABI_X86 => AndroidTargetArch.X86, - _ => throw new NotSupportedException($"Unsupported ABI in store version: 0x{header.version:x}") - }; + TargetArch = (header.version & ASSEMBLY_STORE_ABI_MASK) switch + { + ASSEMBLY_STORE_ABI_AARCH64 => AndroidTargetArch.Arm64, + ASSEMBLY_STORE_ABI_ARM => AndroidTargetArch.Arm, + ASSEMBLY_STORE_ABI_X64 => AndroidTargetArch.X86_64, + ASSEMBLY_STORE_ABI_X86 => AndroidTargetArch.X86, + _ => throw new NotSupportedException($"Unsupported ABI in store version: 0x{header.version:x}") + }; - Is64Bit = (header.version & ASSEMBLY_STORE_FORMAT_VERSION_MASK) != 0; - AssemblyCount = header.entry_count; - IndexEntryCount = header.index_entry_count; + Is64Bit = (header.version & ASSEMBLY_STORE_FORMAT_VERSION_MASK) != 0; + AssemblyCount = header.entry_count; + IndexEntryCount = header.index_entry_count; - StoreStream.Seek((long)elfOffset + Header.NativeSize, SeekOrigin.Begin); - using var reader = CreateReader(); + StoreStream.Seek((long)elfOffset + Header.NativeSize, SeekOrigin.Begin); + using var reader = CreateReader(); - var index = new List(); - for (uint i = 0; i < header.index_entry_count; i++) - { - ulong name_hash; - if (Is64Bit) - { - name_hash = reader.ReadUInt64(); - } - else + var index = new List(); + for (uint i = 0; i < header.index_entry_count; i++) { - name_hash = (ulong)reader.ReadUInt32(); - } + ulong name_hash; + if (Is64Bit) + { + name_hash = reader.ReadUInt64(); + } + else + { + name_hash = (ulong)reader.ReadUInt32(); + } - uint descriptor_index = reader.ReadUInt32(); + uint descriptor_index = reader.ReadUInt32(); #if NET10_0_OR_GREATER bool ignore = reader.ReadByte () != 0; #else - bool ignore = false; + bool ignore = false; #endif - index.Add(new IndexEntry(name_hash, descriptor_index, ignore)); - } + index.Add(new IndexEntry(name_hash, descriptor_index, ignore)); + } - var descriptors = new List(); - for (uint i = 0; i < header.entry_count; i++) - { - uint mapping_index = reader.ReadUInt32(); - uint data_offset = reader.ReadUInt32(); - uint data_size = reader.ReadUInt32(); - uint debug_data_offset = reader.ReadUInt32(); - uint debug_data_size = reader.ReadUInt32(); - uint config_data_offset = reader.ReadUInt32(); - uint config_data_size = reader.ReadUInt32(); - - var desc = new EntryDescriptor + var descriptors = new List(); + for (uint i = 0; i < header.entry_count; i++) { - mapping_index = mapping_index, - data_offset = data_offset, - data_size = data_size, - debug_data_offset = debug_data_offset, - debug_data_size = debug_data_size, - config_data_offset = config_data_offset, - config_data_size = config_data_size, - }; - descriptors.Add(desc); - } + uint mapping_index = reader.ReadUInt32(); + uint data_offset = reader.ReadUInt32(); + uint data_size = reader.ReadUInt32(); + uint debug_data_offset = reader.ReadUInt32(); + uint debug_data_size = reader.ReadUInt32(); + uint config_data_offset = reader.ReadUInt32(); + uint config_data_size = reader.ReadUInt32(); + + var desc = new EntryDescriptor + { + mapping_index = mapping_index, + data_offset = data_offset, + data_size = data_size, + debug_data_offset = debug_data_offset, + debug_data_size = debug_data_size, + config_data_offset = config_data_offset, + config_data_size = config_data_size, + }; + descriptors.Add(desc); + } - var names = new List(); - for (uint i = 0; i < header.entry_count; i++) - { - uint name_length = reader.ReadUInt32(); - byte[] name_bytes = reader.ReadBytes((int)name_length); - names.Add(Encoding.UTF8.GetString(name_bytes)); - } + var names = new List(); + for (uint i = 0; i < header.entry_count; i++) + { + uint name_length = reader.ReadUInt32(); + byte[] name_bytes = reader.ReadBytes((int)name_length); + names.Add(Encoding.UTF8.GetString(name_bytes)); + } - var tempItems = new Dictionary(); - foreach (IndexEntry ie in index) - { - if (!tempItems.TryGetValue(ie.descriptor_index, out TemporaryItem? item)) + var tempItems = new Dictionary(); + foreach (IndexEntry ie in index) { - item = new TemporaryItem(names[(int)ie.descriptor_index], descriptors[(int)ie.descriptor_index], ie.ignore); - tempItems.Add(ie.descriptor_index, item); + if (!tempItems.TryGetValue(ie.descriptor_index, out TemporaryItem? item)) + { + item = new TemporaryItem(names[(int)ie.descriptor_index], descriptors[(int)ie.descriptor_index], ie.ignore); + tempItems.Add(ie.descriptor_index, item); + } + item.IndexEntries.Add(ie); } - item.IndexEntries.Add(ie); - } - if (tempItems.Count != descriptors.Count) - { - throw new InvalidOperationException($"Assembly store '{StorePath}' index is corrupted."); - } + if (tempItems.Count != descriptors.Count) + { + throw new InvalidOperationException($"Assembly store '{StorePath}' index is corrupted."); + } - var storeItems = new List(); - foreach (var kvp in tempItems) - { - TemporaryItem ti = kvp.Value; - var item = new StoreItemV2(TargetArch, ti.Name, Is64Bit, ti.IndexEntries, ti.Descriptor, ti.Ignored); - storeItems.Add(item); + var storeItems = new List(); + foreach (var kvp in tempItems) + { + TemporaryItem ti = kvp.Value; + var item = new StoreItemV2(TargetArch, ti.Name, Is64Bit, ti.IndexEntries, ti.Descriptor, ti.Ignored); + storeItems.Add(item); + } + Assemblies = storeItems.AsReadOnly(); } - Assemblies = storeItems.AsReadOnly(); } } diff --git a/test/Sentry.Android.AssemblyReader.Tests/StoreReaderTests.cs b/test/Sentry.Android.AssemblyReader.Tests/StoreReaderTests.cs new file mode 100644 index 0000000000..5485fd9d95 --- /dev/null +++ b/test/Sentry.Android.AssemblyReader.Tests/StoreReaderTests.cs @@ -0,0 +1,50 @@ +using Sentry.Android.AssemblyReader.V2; + +namespace Sentry.Android.AssemblyReader.Tests; + +public class StoreReaderTests +{ + [Fact] + public void IsSupported_Concurrent_IsThreadSafe() + { + // Arrange + var buffer = new byte[1024 * 1024]; + var memoryStream = new MemoryStream(buffer); + var storeReader = new StoreReader(memoryStream, "testStore", null); + + // Act + Parallel.For(0, 10, _ => storeReader.IsSupported()); + + // No Assert - test passes if no exceptions are thrown + } + + [Fact] + public void ReadEntryImageData_Concurrent_IsThreadSafe() + { + // Arrange + var buffer = new byte[1024 * 1024]; + var memoryStream = new MemoryStream(buffer); + var storeReader = new StoreReader(memoryStream, "testStore", null); + var entry = new StoreReader.StoreItemV2( + AndroidTargetArch.Arm64, + "testAssembly.dll", + is64Bit: true, + new List(), + new StoreReader.EntryDescriptor + { + data_offset = 0, + data_size = 0, + debug_data_offset = 0, + debug_data_size = 0, + config_data_offset = 0, + config_data_size = 0, + mapping_index = 0 + }, + ignore: false); + + // Act + Parallel.For(0, 10, _ => storeReader.ReadEntryImageData(entry)); + + // No Assert - test passes if no exceptions are thrown + } +}