summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLin Lee <linlee@google.com>2023-08-07 09:34:41 +0000
committerAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2023-11-09 21:21:58 +0000
commit5fd7ff8db341cc1794b1029f1f90d521af4f03ab (patch)
treeeef579ce1881ea3b2dfc1c3139602cf860bd8d5e
parent0b149da0e1ca74a753ba92dae7a479769fe61593 (diff)
downloadnetd-5fd7ff8db341cc1794b1029f1f90d521af4f03ab.tar.gz
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
-rw-r--r--server/MDnsSdListener.cpp35
-rw-r--r--server/MDnsSdListener.h4
2 files changed, 26 insertions, 13 deletions
diff --git a/server/MDnsSdListener.cpp b/server/MDnsSdListener.cpp
index d2422f6b..7c9ceff2 100644
--- a/server/MDnsSdListener.cpp
+++ b/server/MDnsSdListener.cpp
@@ -31,6 +31,7 @@
#include <sys/poll.h>
#include <sys/socket.h>
#include <sys/types.h>
+#include <thread>
#define LOG_TAG "MDnsDS"
#define DBG 1
@@ -376,12 +377,18 @@ MDnsSdListener::Monitor::Monitor() {
mPollSize = 10;
socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, mCtrlSocketPair);
- const int rval = ::android::netdutils::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
static int wait_for_property(const char *name, const char *desired_value, int maxwait)
{
@@ -461,14 +468,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 f9c2e87e..8f049126 100644
--- a/server/MDnsSdListener.h
+++ b/server/MDnsSdListener.h
@@ -22,6 +22,7 @@
#include <sysutils/FrameworkListener.h>
#include <mutex>
#include <string>
+#include <thread>
#include "NetdCommand.h"
@@ -83,7 +84,7 @@ class MDnsSdListener {
class Monitor {
public:
Monitor();
- virtual ~Monitor() {}
+ ~Monitor();
DNSServiceRef *allocateServiceRef(int id, Context *c);
void startMonitoring(int id);
DNSServiceRef *lookupServiceRef(int id);
@@ -114,6 +115,7 @@ class MDnsSdListener {
int mPollSize;
int mCtrlSocketPair[2];
std::mutex mMutex;
+ std::thread* mRescanThread;
};
Monitor mMonitor;
};