From f67308a983a2a7d8f4645702d34fba36e41b0c56 Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Tue, 26 Jun 2018 11:16:35 +0000 Subject: [PATCH] Handle invalid dill files more gracefully Instead of bailing out with FATAL et al (which shuts down the VM), give an ApiError when the dill file is invalid. Bug: #33577 Change-Id: I8354b4e68ee95e36584284fd15b3abab3c3bf980 Reviewed-on: https://dart-review.googlesource.com/61937 Commit-Queue: Jens Johansen Reviewed-by: Vyacheslav Egorov --- runtime/vm/bootstrap.cc | 13 ++++++-- runtime/vm/bootstrap_nocore.cc | 13 ++++++-- runtime/vm/dart_api_impl.cc | 12 ++++++-- runtime/vm/kernel.h | 6 ++-- runtime/vm/kernel_binary.cc | 54 +++++++++++++++++++++++++++++----- 5 files changed, 83 insertions(+), 15 deletions(-) diff --git a/runtime/vm/bootstrap.cc b/runtime/vm/bootstrap.cc index f2a7a10c629..cf862abb3f8 100644 --- a/runtime/vm/bootstrap.cc +++ b/runtime/vm/bootstrap.cc @@ -314,8 +314,17 @@ static RawError* BootstrapFromKernel(Thread* thread, const uint8_t* kernel_buffer, intptr_t kernel_buffer_size) { Zone* zone = thread->zone(); - kernel::Program* program = - kernel::Program::ReadFromBuffer(kernel_buffer, kernel_buffer_size); + const char* error = nullptr; + kernel::Program* program = kernel::Program::ReadFromBuffer( + kernel_buffer, kernel_buffer_size, &error); + if (program == nullptr) { + const intptr_t kMessageBufferSize = 512; + char message_buffer[kMessageBufferSize]; + Utils::SNPrint(message_buffer, kMessageBufferSize, + "Can't load Kernel binary: %s.", error); + const String& msg = String::Handle(String::New(message_buffer, Heap::kOld)); + return ApiError::New(msg, Heap::kOld); + } kernel::KernelLoader loader(program); Isolate* isolate = thread->isolate(); diff --git a/runtime/vm/bootstrap_nocore.cc b/runtime/vm/bootstrap_nocore.cc index eadcb765192..2b3d6b567a3 100644 --- a/runtime/vm/bootstrap_nocore.cc +++ b/runtime/vm/bootstrap_nocore.cc @@ -74,8 +74,17 @@ RawError* BootstrapFromKernel(Thread* thread, const uint8_t* kernel_buffer, intptr_t kernel_buffer_size) { Zone* zone = thread->zone(); - kernel::Program* program = - kernel::Program::ReadFromBuffer(kernel_buffer, kernel_buffer_size); + const char* error = nullptr; + kernel::Program* program = kernel::Program::ReadFromBuffer( + kernel_buffer, kernel_buffer_size, &error); + if (program == nullptr) { + const intptr_t kMessageBufferSize = 512; + char message_buffer[kMessageBufferSize]; + Utils::SNPrint(message_buffer, kMessageBufferSize, + "Can't load Kernel binary: %s.", error); + const String& msg = String::Handle(String::New(message_buffer, Heap::kOld)); + return ApiError::New(msg, Heap::kOld); + } kernel::KernelLoader loader(program); Isolate* isolate = thread->isolate(); diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index 5ef1eabffce..2ccdeca7c9b 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -5384,8 +5384,12 @@ DART_EXPORT Dart_Handle Dart_LoadScriptFromKernel(const uint8_t* buffer, CHECK_CALLBACK_STATE(T); CHECK_COMPILATION_ALLOWED(I); + const char* error = nullptr; kernel::Program* program = - kernel::Program::ReadFromBuffer(buffer, buffer_size); + kernel::Program::ReadFromBuffer(buffer, buffer_size, &error); + if (program == nullptr) { + return Api::NewError("Can't load Kernel binary: %s.", error); + } const Object& tmp = kernel::KernelLoader::LoadEntireProgram(program); delete program; @@ -5681,8 +5685,12 @@ DART_EXPORT Dart_Handle Dart_LoadLibraryFromKernel(const uint8_t* buffer, CHECK_CALLBACK_STATE(T); CHECK_COMPILATION_ALLOWED(I); + const char* error = nullptr; kernel::Program* program = - kernel::Program::ReadFromBuffer(buffer, buffer_size); + kernel::Program::ReadFromBuffer(buffer, buffer_size, &error); + if (program == nullptr) { + return Api::NewError("Can't load Kernel binary: %s.", error); + } const Object& result = kernel::KernelLoader::LoadEntireProgram(program, false); delete program; diff --git a/runtime/vm/kernel.h b/runtime/vm/kernel.h index ac0d376724e..5bcf9192fc9 100644 --- a/runtime/vm/kernel.h +++ b/runtime/vm/kernel.h @@ -60,10 +60,12 @@ class Program { // Read a kernel Program from the given Reader. Note the returned Program // can potentially contain several "sub programs", though the library count // etc will reference the last "sub program" only. - static Program* ReadFrom(Reader* reader); + static Program* ReadFrom(Reader* reader, const char** error = nullptr); static Program* ReadFromFile(const char* script_uri); - static Program* ReadFromBuffer(const uint8_t* buffer, intptr_t buffer_length); + static Program* ReadFromBuffer(const uint8_t* buffer, + intptr_t buffer_length, + const char** error = nullptr); bool is_single_program() { return single_program_; } NameIndex main_method() { return main_method_reference_; } diff --git a/runtime/vm/kernel_binary.cc b/runtime/vm/kernel_binary.cc index ec88057fa0c..17128cecbf5 100644 --- a/runtime/vm/kernel_binary.cc +++ b/runtime/vm/kernel_binary.cc @@ -29,14 +29,49 @@ const char* Reader::TagName(Tag tag) { return "Unknown"; } -Program* Program::ReadFrom(Reader* reader) { +const char* kKernelInvalidFilesize = + "File size is too small to a valid kernel file"; +const char* kKernelInvalidMagicIdentifier = "Invalid magic identifier"; +const char* kKernelInvalidBinaryFormatVersion = + "Invalid kernel binary format version"; +const char* kKernelInvalidSizeIndicated = + "Invalid kernel binary: Indicated size is invalid"; + +Program* Program::ReadFrom(Reader* reader, const char** error) { + if (reader->size() < 59) { + // A kernel file currently contains at least the following: + // * Magic number (32) + // * Kernel version (32) + // * Length of source map (32) + // * Length of canonical name table (8) + // * Metadata length (32) + // * Length of string table (8) + // * Length of constant table (8) + // * Component index (10 * 32) + // + // so is at least 59 bytes. + // (Technically it will also contain an empty entry in both source map and + // string table, taking up another 8 bytes.) + if (error != nullptr) { + *error = kKernelInvalidFilesize; + } + return nullptr; + } + uint32_t magic = reader->ReadUInt32(); - if (magic != kMagicProgramFile) FATAL("Invalid magic identifier"); + if (magic != kMagicProgramFile) { + if (error != nullptr) { + *error = kKernelInvalidMagicIdentifier; + } + return nullptr; + } uint32_t formatVersion = reader->ReadUInt32(); if (formatVersion != kBinaryFormatVersion) { - FATAL2("Invalid kernel binary format version (found %u, expected %u)", - formatVersion, kBinaryFormatVersion); + if (error != nullptr) { + *error = kKernelInvalidBinaryFormatVersion; + } + return nullptr; } Program* program = new Program(); @@ -51,7 +86,11 @@ Program* Program::ReadFrom(Reader* reader) { intptr_t size = reader->ReadUInt32(); intptr_t start = reader->offset() - size; if (start < 0) { - FATAL("Invalid kernel binary: Indicated size is invalid."); + if (error != nullptr) { + *error = kKernelInvalidSizeIndicated; + } + delete program; + return nullptr; } ++subprogram_count; if (subprogram_count > 1) break; @@ -116,9 +155,10 @@ Program* Program::ReadFromFile(const char* script_uri) { } Program* Program::ReadFromBuffer(const uint8_t* buffer, - intptr_t buffer_length) { + intptr_t buffer_length, + const char** error) { kernel::Reader reader(buffer, buffer_length); - return kernel::Program::ReadFrom(&reader); + return kernel::Program::ReadFrom(&reader, error); } } // namespace kernel