From 40da03b74242cf6c2d201dcd213cac1ec57847b1 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 28 Jun 2019 13:21:27 -0700 Subject: [PATCH] Revert "logd: validate and fill in socket credentials" We don't want to fake socket credentials if they were not provided by the kernel. If there is a bug preventing us from reading the credentials then it must be solved directly. Test: logd, liblog unit tests Test: boot and ensure overflow uid doesn't show up This reverts commit c4e4823b00a94627e922eada1172688818471b0c. Change-Id: I683129a8a214637635f163ae25c39bb8a47cd50f --- logd/LogBuffer.h | 7 +------ logd/LogBufferInterface.cpp | 10 +--------- logd/LogBufferInterface.h | 3 --- logd/LogListener.cpp | 28 +--------------------------- logd/LogListener.h | 10 ---------- logd/LogStatistics.cpp | 23 ----------------------- logd/LogStatistics.h | 12 ++++-------- logd/LogUtils.h | 2 -- 8 files changed, 7 insertions(+), 88 deletions(-) diff --git a/logd/LogBuffer.h b/logd/LogBuffer.h index 774d4ab0b..404433fb6 100644 --- a/logd/LogBuffer.h +++ b/logd/LogBuffer.h @@ -159,12 +159,7 @@ class LogBuffer : public LogBufferInterface { const char* pidToName(pid_t pid) { return stats.pidToName(pid); } - virtual uid_t pidToUid(pid_t pid) override { - return stats.pidToUid(pid); - } - virtual pid_t tidToPid(pid_t tid) override { - return stats.tidToPid(tid); - } + uid_t pidToUid(pid_t pid) { return stats.pidToUid(pid); } const char* uidToName(uid_t uid) { return stats.uidToName(uid); } diff --git a/logd/LogBufferInterface.cpp b/logd/LogBufferInterface.cpp index 4b6d36316..66b3ab420 100644 --- a/logd/LogBufferInterface.cpp +++ b/logd/LogBufferInterface.cpp @@ -15,15 +15,7 @@ */ #include "LogBufferInterface.h" -#include "LogUtils.h" LogBufferInterface::LogBufferInterface() { } -LogBufferInterface::~LogBufferInterface() { -} -uid_t LogBufferInterface::pidToUid(pid_t pid) { - return android::pidToUid(pid); -} -pid_t LogBufferInterface::tidToPid(pid_t tid) { - return android::tidToPid(tid); -} +LogBufferInterface::~LogBufferInterface() {} \ No newline at end of file diff --git a/logd/LogBufferInterface.h b/logd/LogBufferInterface.h index f31e24457..2bb08f926 100644 --- a/logd/LogBufferInterface.h +++ b/logd/LogBufferInterface.h @@ -33,9 +33,6 @@ class LogBufferInterface { virtual int log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid, pid_t tid, const char* msg, uint16_t len) = 0; - virtual uid_t pidToUid(pid_t pid); - virtual pid_t tidToPid(pid_t tid); - private: DISALLOW_COPY_AND_ASSIGN(LogBufferInterface); }; diff --git a/logd/LogListener.cpp b/logd/LogListener.cpp index 2f227788c..7f78e1991 100644 --- a/logd/LogListener.cpp +++ b/logd/LogListener.cpp @@ -14,9 +14,7 @@ * limitations under the License. */ -#include #include -#include #include #include #include @@ -78,11 +76,8 @@ bool LogListener::onDataAvailable(SocketClient* cli) { cmsg = CMSG_NXTHDR(&hdr, cmsg); } - struct ucred fake_cred; if (cred == nullptr) { - cred = &fake_cred; - cred->pid = 0; - cred->uid = DEFAULT_OVERFLOWUID; + return false; } if (cred->uid == AID_LOGD) { @@ -106,27 +101,6 @@ bool LogListener::onDataAvailable(SocketClient* cli) { return false; } - // Check credential validity, acquire corrected details if not supplied. - if (cred->pid == 0) { - cred->pid = logbuf ? logbuf->tidToPid(header->tid) - : android::tidToPid(header->tid); - if (cred->pid == getpid()) { - // We expect that /proc// is accessible to self even without - // readproc group, so that we will always drop messages that come - // from any of our logd threads and their library calls. - return false; // ignore self - } - } - if (cred->uid == DEFAULT_OVERFLOWUID) { - uid_t uid = - logbuf ? logbuf->pidToUid(cred->pid) : android::pidToUid(cred->pid); - if (uid == AID_LOGD) { - uid = logbuf ? logbuf->pidToUid(header->tid) - : android::pidToUid(cred->pid); - } - if (uid != AID_LOGD) cred->uid = uid; - } - char* msg = ((char*)buffer) + sizeof(android_log_header_t); n -= sizeof(android_log_header_t); diff --git a/logd/LogListener.h b/logd/LogListener.h index a562a54b9..e16c5fb62 100644 --- a/logd/LogListener.h +++ b/logd/LogListener.h @@ -20,16 +20,6 @@ #include #include "LogReader.h" -// DEFAULT_OVERFLOWUID is defined in linux/highuid.h, which is not part of -// the uapi headers for userspace to use. This value is filled in on the -// out-of-band socket credentials if the OS fails to find one available. -// One of the causes of this is if SO_PASSCRED is set, all the packets before -// that point will have this value. We also use it in a fake credential if -// no socket credentials are supplied. -#ifndef DEFAULT_OVERFLOWUID -#define DEFAULT_OVERFLOWUID 65534 -#endif - class LogListener : public SocketListener { LogBufferInterface* logbuf; LogReader* reader; diff --git a/logd/LogStatistics.cpp b/logd/LogStatistics.cpp index 116e08eba..431b7784b 100644 --- a/logd/LogStatistics.cpp +++ b/logd/LogStatistics.cpp @@ -837,35 +837,12 @@ uid_t pidToUid(pid_t pid) { } return AID_LOGD; // associate this with the logger } - -pid_t tidToPid(pid_t tid) { - char buffer[512]; - snprintf(buffer, sizeof(buffer), "/proc/%u/status", tid); - FILE* fp = fopen(buffer, "r"); - if (fp) { - while (fgets(buffer, sizeof(buffer), fp)) { - int pid = tid; - char space = 0; - if ((sscanf(buffer, "Tgid: %d%c", &pid, &space) == 2) && - isspace(space)) { - fclose(fp); - return pid; - } - } - fclose(fp); - } - return tid; -} } uid_t LogStatistics::pidToUid(pid_t pid) { return pidTable.add(pid)->second.getUid(); } -pid_t LogStatistics::tidToPid(pid_t tid) { - return tidTable.add(tid)->second.getPid(); -} - // caller must free character string const char* LogStatistics::pidToName(pid_t pid) const { // An inconvenient truth ... getName() can alter the object diff --git a/logd/LogStatistics.h b/logd/LogStatistics.h index 469f6dcf4..0782de3d1 100644 --- a/logd/LogStatistics.h +++ b/logd/LogStatistics.h @@ -306,6 +306,10 @@ struct UidEntry : public EntryBaseDropped { std::string format(const LogStatistics& stat, log_id_t id) const; }; +namespace android { +uid_t pidToUid(pid_t pid); +} + struct PidEntry : public EntryBaseDropped { const pid_t pid; uid_t uid; @@ -385,13 +389,6 @@ struct TidEntry : public EntryBaseDropped { uid(android::pidToUid(tid)), name(android::tidToName(tid)) { } - TidEntry(pid_t tid) - : EntryBaseDropped(), - tid(tid), - pid(android::tidToPid(tid)), - uid(android::pidToUid(tid)), - name(android::tidToName(tid)) { - } explicit TidEntry(const LogBufferElement* element) : EntryBaseDropped(element), tid(element->getTid()), @@ -787,7 +784,6 @@ class LogStatistics { // helper (must be locked directly or implicitly by mLogElementsLock) const char* pidToName(pid_t pid) const; uid_t pidToUid(pid_t pid); - pid_t tidToPid(pid_t tid); const char* uidToName(uid_t uid) const; }; diff --git a/logd/LogUtils.h b/logd/LogUtils.h index 4dcd3e7f9..fa9f39895 100644 --- a/logd/LogUtils.h +++ b/logd/LogUtils.h @@ -38,8 +38,6 @@ size_t sizesTotal(); // Caller must own and free returned value char* pidToName(pid_t pid); char* tidToName(pid_t tid); -uid_t pidToUid(pid_t pid); -pid_t tidToPid(pid_t tid); // Furnished in LogTags.cpp. Thread safe. const char* tagToName(uint32_t tag);