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>
This commit is contained in:
parent
bfde407de5
commit
7b981b2135
3 changed files with 12 additions and 11 deletions
|
@ -68,7 +68,7 @@ bool ModuleComparer::Compare(const string& symbol_data) {
|
||||||
buffer.reset();
|
buffer.reset();
|
||||||
|
|
||||||
// Serialize BasicSourceLineResolver::Module.
|
// Serialize BasicSourceLineResolver::Module.
|
||||||
unsigned int serialized_size = 0;
|
size_t serialized_size = 0;
|
||||||
scoped_array<char> serialized_data(
|
scoped_array<char> serialized_data(
|
||||||
serializer_.Serialize(*(basic_module.get()), &serialized_size));
|
serializer_.Serialize(*(basic_module.get()), &serialized_size));
|
||||||
ASSERT_TRUE(serialized_data.get());
|
ASSERT_TRUE(serialized_data.get());
|
||||||
|
|
|
@ -111,10 +111,10 @@ char* ModuleSerializer::Write(const BasicSourceLineResolver::Module& module,
|
||||||
return dest;
|
return dest;
|
||||||
}
|
}
|
||||||
|
|
||||||
char* ModuleSerializer::Serialize(
|
char* ModuleSerializer::Serialize(const BasicSourceLineResolver::Module& module,
|
||||||
const BasicSourceLineResolver::Module& module, unsigned int* size) {
|
size_t* size) {
|
||||||
// Compute size of memory to allocate.
|
// 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.
|
// Allocate memory for serialized data.
|
||||||
char* serialized_data = new char[size_to_alloc];
|
char* serialized_data = new char[size_to_alloc];
|
||||||
|
@ -128,8 +128,8 @@ char* ModuleSerializer::Serialize(
|
||||||
// Write serialized data to allocated memory chunk.
|
// Write serialized data to allocated memory chunk.
|
||||||
char* end_address = Write(module, serialized_data);
|
char* end_address = Write(module, serialized_data);
|
||||||
// Verify the allocated memory size is equal to the size of data been written.
|
// Verify the allocated memory size is equal to the size of data been written.
|
||||||
unsigned int size_written =
|
const size_t size_written =
|
||||||
static_cast<unsigned int>(end_address - serialized_data);
|
static_cast<size_t>(end_address - serialized_data);
|
||||||
if (size_to_alloc != size_written) {
|
if (size_to_alloc != size_written) {
|
||||||
BPLOG(ERROR) << "size_to_alloc differs from size_written: "
|
BPLOG(ERROR) << "size_to_alloc differs from size_written: "
|
||||||
<< size_to_alloc << " vs " << 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.
|
// Set size and return the start address of memory chunk.
|
||||||
if (size)
|
if (size)
|
||||||
*size = size_to_alloc;
|
*size = size_to_alloc;
|
||||||
|
|
||||||
return serialized_data;
|
return serialized_data;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -150,7 +151,7 @@ bool ModuleSerializer::SerializeModuleAndLoadIntoFastResolver(
|
||||||
BasicSourceLineResolver::Module* basic_module =
|
BasicSourceLineResolver::Module* basic_module =
|
||||||
dynamic_cast<BasicSourceLineResolver::Module*>(iter->second);
|
dynamic_cast<BasicSourceLineResolver::Module*>(iter->second);
|
||||||
|
|
||||||
unsigned int size = 0;
|
size_t size = 0;
|
||||||
scoped_array<char> symbol_data(Serialize(*basic_module, &size));
|
scoped_array<char> symbol_data(Serialize(*basic_module, &size));
|
||||||
if (!symbol_data.get()) {
|
if (!symbol_data.get()) {
|
||||||
BPLOG(ERROR) << "Serialization failed for module: " << basic_module->name_;
|
BPLOG(ERROR) << "Serialization failed for module: " << basic_module->name_;
|
||||||
|
@ -201,8 +202,8 @@ bool ModuleSerializer::ConvertOneModule(
|
||||||
return SerializeModuleAndLoadIntoFastResolver(iter, fast_resolver);
|
return SerializeModuleAndLoadIntoFastResolver(iter, fast_resolver);
|
||||||
}
|
}
|
||||||
|
|
||||||
char* ModuleSerializer::SerializeSymbolFileData(
|
char* ModuleSerializer::SerializeSymbolFileData(const string& symbol_data,
|
||||||
const string& symbol_data, unsigned int* size) {
|
size_t* size) {
|
||||||
scoped_ptr<BasicSourceLineResolver::Module> module(
|
scoped_ptr<BasicSourceLineResolver::Module> module(
|
||||||
new BasicSourceLineResolver::Module("no name"));
|
new BasicSourceLineResolver::Module("no name"));
|
||||||
scoped_array<char> buffer(new char[symbol_data.size() + 1]);
|
scoped_array<char> buffer(new char[symbol_data.size() + 1]);
|
||||||
|
|
|
@ -72,13 +72,13 @@ class ModuleSerializer {
|
||||||
// Caller takes the ownership of the memory chunk (allocated on heap), and
|
// Caller takes the ownership of the memory chunk (allocated on heap), and
|
||||||
// owner should call delete [] to free the memory after use.
|
// owner should call delete [] to free the memory after use.
|
||||||
char* Serialize(const BasicSourceLineResolver::Module& module,
|
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.
|
// Given the string format symbol_data, produces a chunk of serialized data.
|
||||||
// Caller takes ownership of the serialized data (on heap), and owner should
|
// Caller takes ownership of the serialized data (on heap), and owner should
|
||||||
// call delete [] to free the memory after use.
|
// call delete [] to free the memory after use.
|
||||||
char* SerializeSymbolFileData(const string& symbol_data,
|
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
|
// Serializes one loaded module with given moduleid in the basic source line
|
||||||
// resolver, and loads the serialized data into the fast source line resolver.
|
// resolver, and loads the serialized data into the fast source line resolver.
|
||||||
|
|
Loading…
Reference in a new issue