From 7b981b213550b08530ff17386c1c29c9771be40b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Wed, 26 Apr 2023 13:20:27 -0700 Subject: [PATCH] Replace unsigned int with size_t for ModuleSerializer This is a speculative fix for a memory bug where our symbol files are looking like they've grown enough that serializing them will outgrow UINT_MAX. Before this change a size_t is implicitly cast to a size_t in unsigned int, allocate a buffer of that size and then continue to write module data out of bounds. I have not been able to reproduce the OOB write locally as the original uploaded symbol data is gone, but I have been able to reproduce builds where, if we enable inline frames and CFI dumping, the size grows to 3.6GB when serializing it, which is close enough to 4.2GB that the wrapping theory seems reasonable on another board or build. No effort is made here to prevent wrapping behavior on 32-bit systems. Bug: b/237242489, chromium:1410232 Change-Id: I3d7ec03c51c298f10df3d5b1e5306433875c7919 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4477821 Reviewed-by: Leonard Grey Reviewed-by: Mark Mentovai --- src/processor/module_comparer.cc | 2 +- src/processor/module_serializer.cc | 17 +++++++++-------- src/processor/module_serializer.h | 4 ++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/processor/module_comparer.cc b/src/processor/module_comparer.cc index 1bf0b316..a6413038 100644 --- a/src/processor/module_comparer.cc +++ b/src/processor/module_comparer.cc @@ -68,7 +68,7 @@ bool ModuleComparer::Compare(const string& symbol_data) { buffer.reset(); // Serialize BasicSourceLineResolver::Module. - unsigned int serialized_size = 0; + size_t serialized_size = 0; scoped_array serialized_data( serializer_.Serialize(*(basic_module.get()), &serialized_size)); ASSERT_TRUE(serialized_data.get()); diff --git a/src/processor/module_serializer.cc b/src/processor/module_serializer.cc index 91d0006e..05519958 100644 --- a/src/processor/module_serializer.cc +++ b/src/processor/module_serializer.cc @@ -111,10 +111,10 @@ char* ModuleSerializer::Write(const BasicSourceLineResolver::Module& module, return dest; } -char* ModuleSerializer::Serialize( - const BasicSourceLineResolver::Module& module, unsigned int* size) { +char* ModuleSerializer::Serialize(const BasicSourceLineResolver::Module& module, + size_t* size) { // Compute size of memory to allocate. - unsigned int size_to_alloc = SizeOf(module); + const size_t size_to_alloc = SizeOf(module); // Allocate memory for serialized data. char* serialized_data = new char[size_to_alloc]; @@ -128,8 +128,8 @@ char* ModuleSerializer::Serialize( // Write serialized data to allocated memory chunk. char* end_address = Write(module, serialized_data); // Verify the allocated memory size is equal to the size of data been written. - unsigned int size_written = - static_cast(end_address - serialized_data); + const size_t size_written = + static_cast(end_address - serialized_data); if (size_to_alloc != size_written) { BPLOG(ERROR) << "size_to_alloc differs from size_written: " << size_to_alloc << " vs " << size_written; @@ -138,6 +138,7 @@ char* ModuleSerializer::Serialize( // Set size and return the start address of memory chunk. if (size) *size = size_to_alloc; + return serialized_data; } @@ -150,7 +151,7 @@ bool ModuleSerializer::SerializeModuleAndLoadIntoFastResolver( BasicSourceLineResolver::Module* basic_module = dynamic_cast(iter->second); - unsigned int size = 0; + size_t size = 0; scoped_array symbol_data(Serialize(*basic_module, &size)); if (!symbol_data.get()) { BPLOG(ERROR) << "Serialization failed for module: " << basic_module->name_; @@ -201,8 +202,8 @@ bool ModuleSerializer::ConvertOneModule( return SerializeModuleAndLoadIntoFastResolver(iter, fast_resolver); } -char* ModuleSerializer::SerializeSymbolFileData( - const string& symbol_data, unsigned int* size) { +char* ModuleSerializer::SerializeSymbolFileData(const string& symbol_data, + size_t* size) { scoped_ptr module( new BasicSourceLineResolver::Module("no name")); scoped_array buffer(new char[symbol_data.size() + 1]); diff --git a/src/processor/module_serializer.h b/src/processor/module_serializer.h index 4e365a41..fd387cbb 100644 --- a/src/processor/module_serializer.h +++ b/src/processor/module_serializer.h @@ -72,13 +72,13 @@ class ModuleSerializer { // Caller takes the ownership of the memory chunk (allocated on heap), and // owner should call delete [] to free the memory after use. char* Serialize(const BasicSourceLineResolver::Module& module, - unsigned int* size = NULL); + size_t* size = nullptr); // Given the string format symbol_data, produces a chunk of serialized data. // Caller takes ownership of the serialized data (on heap), and owner should // call delete [] to free the memory after use. char* SerializeSymbolFileData(const string& symbol_data, - unsigned int* size = NULL); + size_t* size = nullptr); // Serializes one loaded module with given moduleid in the basic source line // resolver, and loads the serialized data into the fast source line resolver.