diff options
author | Flash Liu <flash.liu@mediatek.com> | 2019-10-31 11:18:55 +0800 |
---|---|---|
committer | Treehugger Robot <treehugger-gerrit@google.com> | 2020-02-13 23:16:48 +0000 |
commit | 013e208bddd2ea316390429113ca1f43b3924ac2 (patch) | |
tree | 86b7c6136adae543040c874bc703dc37a236267f | |
parent | e1a445158f65ad7eb47deee7f0fa580e34e836df (diff) | |
download | platform_art-013e208bddd2ea316390429113ca1f43b3924ac2.tar.gz platform_art-013e208bddd2ea316390429113ca1f43b3924ac2.tar.bz2 platform_art-013e208bddd2ea316390429113ca1f43b3924ac2.zip |
Make AdbConnectionState statically
Asan detected UAF on runtime-callbacks.
This makes AdbConnectionState allocated on plugin-init
and does not remove on callback.
Bug: 143739105
Test: boot device with adbconnection on asan load
Test: atest CtsJdwpTunnelHostTestCases
Change-Id: Ia5a547235f23ad12ead97186b4d1dfa2f08abb63
-rw-r--r-- | adbconnection/adbconnection.cc | 23 | ||||
-rw-r--r-- | adbconnection/adbconnection.h | 1 |
2 files changed, 12 insertions, 12 deletions
diff --git a/adbconnection/adbconnection.cc b/adbconnection/adbconnection.cc index c512b0678be..cdf97141a03 100644 --- a/adbconnection/adbconnection.cc +++ b/adbconnection/adbconnection.cc @@ -83,7 +83,7 @@ static constexpr off_t kPacketCommandOff = 10; static constexpr uint8_t kDdmCommandSet = 199; static constexpr uint8_t kDdmChunkCommand = 1; -static AdbConnectionState* gState; +static std::optional<AdbConnectionState> gState; static bool IsDebuggingPossible() { return art::Dbg::IsJdwpAllowed(); @@ -155,6 +155,15 @@ AdbConnectionState::AdbConnectionState(const std::string& agent_name) art::Runtime::Current()->GetRuntimeCallbacks()->AddDebuggerControlCallback(&controller_); } +AdbConnectionState::~AdbConnectionState() { + // Remove the startup callback. + art::Thread* self = art::Thread::Current(); + if (self != nullptr) { + art::ScopedObjectAccess soa(self); + art::Runtime::Current()->GetRuntimeCallbacks()->RemoveDebuggerControlCallback(&controller_); + } +} + static jobject CreateAdbConnectionThread(art::Thread* thr) { JNIEnv* env = thr->GetJniEnv(); // Move to native state to talk with the jnienv api. @@ -179,7 +188,6 @@ struct CallbackData { static void* CallbackFunction(void* vdata) { std::unique_ptr<CallbackData> data(reinterpret_cast<CallbackData*>(vdata)); - CHECK(data->this_ == gState); art::Thread* self = art::Thread::Attach(kAdbConnectionThreadName, true, data->thr_); @@ -205,10 +213,6 @@ static void* CallbackFunction(void* vdata) { int detach_result = art::Runtime::Current()->GetJavaVM()->DetachCurrentThread(); CHECK_EQ(detach_result, 0); - // Get rid of the connection - gState = nullptr; - delete data->this_; - return nullptr; } @@ -830,17 +834,12 @@ void AdbConnectionState::StopDebuggerThreads() { extern "C" bool ArtPlugin_Initialize() { DCHECK(art::Runtime::Current()->GetJdwpProvider() == art::JdwpProvider::kAdbConnection); // TODO Provide some way for apps to set this maybe? - DCHECK(gState == nullptr); - gState = new AdbConnectionState(kDefaultJdwpAgentName); + gState.emplace(kDefaultJdwpAgentName); return ValidateJdwpOptions(art::Runtime::Current()->GetJdwpOptions()); } extern "C" bool ArtPlugin_Deinitialize() { gState->StopDebuggerThreads(); - if (!gState->DebuggerThreadsStarted()) { - // If debugger threads were started then those threads will delete the state once they are done. - delete gState; - } return true; } diff --git a/adbconnection/adbconnection.h b/adbconnection/adbconnection.h index f9b0928298e..32f42bac60b 100644 --- a/adbconnection/adbconnection.h +++ b/adbconnection/adbconnection.h @@ -75,6 +75,7 @@ struct AdbConnectionDdmCallback : public art::DdmCallback { class AdbConnectionState { public: explicit AdbConnectionState(const std::string& name); + ~AdbConnectionState(); // Called on the listening thread to start dealing with new input. thr is used to attach the new // thread to the runtime. |