diff options
author | Peter Boström <pbos@chromium.org> | 2023-04-26 13:20:27 -0700 |
---|---|---|
committer | Peter Boström <pbos@chromium.org> | 2023-04-26 20:22:23 +0000 |
commit | 7b981b213550b08530ff17386c1c29c9771be40b (patch) | |
tree | 560a26b756a643654fad441756cb081c4c21222d /src | |
parent | bfde407de559c10d6cef861b3873ff287c24e761 (diff) | |
download | google-breakpad-7b981b213550b08530ff17386c1c29c9771be40b.tar.gz |
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 <lgrey@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Diffstat (limited to 'src')
-rw-r--r-- | src/processor/module_comparer.cc | 2 | ||||
-rw-r--r-- | src/processor/module_serializer.cc | 17 | ||||
-rw-r--r-- | 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<char> 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<unsigned int>(end_address - serialized_data); + const size_t size_written = + static_cast<size_t>(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<BasicSourceLineResolver::Module*>(iter->second); - unsigned int size = 0; + size_t size = 0; scoped_array<char> 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<BasicSourceLineResolver::Module> module( new BasicSourceLineResolver::Module("no name")); scoped_array<char> 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. |