From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Lin Lee Date: Mon, 7 Aug 2023 09:34:41 +0000 Subject: [PATCH] Fix Heap-use-after-free in MDnsSdListener::Monitor::run Use thread join to avoid thread exiting after instance recycled. Prior to implementing this patch, fuzzing would lead to a segmentation fault after approximately 500 rounds. With the addition of the patch, the fuzzing process can now be repeated for over 30,000 rounds. Test: m, fuzzing Fuzzing: mma mdns_service_fuzzer && adb sync data && adb shell /data/fuzz/arm64/mdns_service_fuzzer/mdns_service_fuzzer Bug: 272382770 Ignore-AOSP-First: Security Issue (cherry picked from commit 9c0c15f80cffb98b36284dd169a2e62e059dbbe3) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:75e5e2e1faec7aa2812fc6fba30d6fe80558bacd) Merged-In: I5bc85451b4e6539bad45ceb672924a37952cc138 Change-Id: I5bc85451b4e6539bad45ceb672924a37952cc138 --- server/MDnsSdListener.cpp | 36 ++++++++++++++++++++++++------------ server/MDnsSdListener.h | 4 +++- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/server/MDnsSdListener.cpp b/server/MDnsSdListener.cpp index b54014cd..e3dd616d 100644 --- a/server/MDnsSdListener.cpp +++ b/server/MDnsSdListener.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #define LOG_TAG "MDnsDS" #define DBG 1 @@ -524,10 +525,17 @@ MDnsSdListener::Monitor::Monitor() { socketpair(AF_LOCAL, SOCK_STREAM, 0, mCtrlSocketPair); pthread_mutex_init(&mHeadMutex, NULL); - const int rval = ::android::net::threadLaunch(this); - if (rval != 0) { - ALOGW("Error spawning monitor thread: %s (%d)", strerror(-rval), -rval); - } + mRescanThread = new std::thread(&Monitor::run, this); + if (!mRescanThread->joinable()) ALOGE("Unable to launch thread."); +} + +MDnsSdListener::Monitor::~Monitor() { + if (VDBG) ALOGD("Monitor recycling"); + close(mCtrlSocketPair[1]); // interrupt poll in MDnsSdListener::Monitor::run() and revent will + // be 17 = POLLIN | POLLHUP + mRescanThread->join(); + delete mRescanThread; + if (VDBG) ALOGD("Monitor recycled"); } #define NAP_TIME 200 // 200 ms between polls @@ -617,14 +625,18 @@ void MDnsSdListener::Monitor::run() { } } if (VDBG) ALOGD("controlSocket shows revent= %d", mPollFds[0].revents); - switch (mPollFds[0].revents) { - case POLLIN: { - char readBuf[2]; - read(mCtrlSocketPair[0], &readBuf, 1); - if (DBG) ALOGD("MDnsSdListener::Monitor got %c", readBuf[0]); - if (memcmp(RESCAN, readBuf, 1) == 0) { - pollCount = rescan(); - } + if (mPollFds[0].revents & POLLHUP) { + free(mPollFds); + free(mPollRefs); + if (VDBG) ALOGD("Monitor thread leaving."); + return; + } + if (mPollFds[0].revents == POLLIN) { + char readBuf[2]; + read(mCtrlSocketPair[0], &readBuf, 1); + if (DBG) ALOGD("MDnsSdListener::Monitor got %c", readBuf[0]); + if (memcmp(RESCAN, readBuf, 1) == 0) { + pollCount = rescan(); } } mPollFds[0].revents = 0; diff --git a/server/MDnsSdListener.h b/server/MDnsSdListener.h index 8c6096e8..2b3cb5e2 100644 --- a/server/MDnsSdListener.h +++ b/server/MDnsSdListener.h @@ -20,6 +20,7 @@ #include #include #include +#include #include "NetdCommand.h" @@ -71,7 +72,7 @@ private: class Monitor { public: Monitor(); - virtual ~Monitor() {} + ~Monitor(); DNSServiceRef *allocateServiceRef(int id, Context *c); void startMonitoring(int id); DNSServiceRef *lookupServiceRef(int id); @@ -101,6 +102,7 @@ private: int mPollSize; int mCtrlSocketPair[2]; pthread_mutex_t mHeadMutex; + std::thread* mRescanThread; }; class Handler : public NetdCommand {