From c4e4823b00a94627e922eada1172688818471b0c Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Thu, 4 May 2017 13:54:46 -0700 Subject: [PATCH] logd: validate and fill in socket credentials - android::pidToUid() additional checking. Make sure if we have to convert a PID to an UID that the parse of /proc//status requires a trailing space after the number - android::tidToPid() added, in the same vein as android::pidToUid(). - stats.tidToPid() added - If no credentials, set PID to 0 and UID to DEFAULT_OVERFLOWUID - If credentialed PID is 0, use stats.tidToPid() - If credentialed UID is DEFAULT_OVERFLOWUID, use stats.pidToUid() Test: remove +passcred from logd.rc for daemon and confirm very few UID=65534 or PID=0 cases actually show up Bug: 37985222 Change-Id: I7d20506e70e67beb3043d1537cf9450ab58dc278 --- logd/LogBuffer.h | 5 ++++- logd/LogBufferInterface.cpp | 9 ++++++++- logd/LogBufferInterface.h | 3 +++ logd/LogListener.cpp | 28 +++++++++++++++++++++++++++- logd/LogListener.h | 10 ++++++++++ logd/LogStatistics.cpp | 30 ++++++++++++++++++++++++++++-- logd/LogStatistics.h | 12 ++++++++---- logd/LogUtils.h | 2 ++ 8 files changed, 90 insertions(+), 9 deletions(-) diff --git a/logd/LogBuffer.h b/logd/LogBuffer.h index e59775488..f0d6fcb2e 100644 --- a/logd/LogBuffer.h +++ b/logd/LogBuffer.h @@ -159,9 +159,12 @@ class LogBuffer : public LogBufferInterface { const char* pidToName(pid_t pid) { return stats.pidToName(pid); } - uid_t pidToUid(pid_t 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); + } const char* uidToName(uid_t uid) { return stats.uidToName(uid); } diff --git a/logd/LogBufferInterface.cpp b/logd/LogBufferInterface.cpp index 3cb2b898f..4b6d36316 100644 --- a/logd/LogBufferInterface.cpp +++ b/logd/LogBufferInterface.cpp @@ -15,8 +15,15 @@ */ #include "LogBufferInterface.h" +#include "LogUtils.h" LogBufferInterface::LogBufferInterface() { } LogBufferInterface::~LogBufferInterface() { -} \ No newline at end of file +} +uid_t LogBufferInterface::pidToUid(pid_t pid) { + return android::pidToUid(pid); +} +pid_t LogBufferInterface::tidToPid(pid_t tid) { + return android::tidToPid(tid); +} diff --git a/logd/LogBufferInterface.h b/logd/LogBufferInterface.h index 7d82b91f3..ff73a227c 100644 --- a/logd/LogBufferInterface.h +++ b/logd/LogBufferInterface.h @@ -33,6 +33,9 @@ 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, unsigned short 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 3c0d08dc6..d2df68eef 100644 --- a/logd/LogListener.cpp +++ b/logd/LogListener.cpp @@ -14,7 +14,9 @@ * limitations under the License. */ +#include #include +#include #include #include #include @@ -72,8 +74,11 @@ bool LogListener::onDataAvailable(SocketClient* cli) { cmsg = CMSG_NXTHDR(&hdr, cmsg); } + struct ucred fake_cred; if (cred == NULL) { - return false; + cred = &fake_cred; + cred->pid = 0; + cred->uid = DEFAULT_OVERFLOWUID; } if (cred->uid == AID_LOGD) { @@ -96,6 +101,27 @@ 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 e16c5fb62..a562a54b9 100644 --- a/logd/LogListener.h +++ b/logd/LogListener.h @@ -20,6 +20,16 @@ #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 d20d90e58..af59ddc25 100644 --- a/logd/LogStatistics.cpp +++ b/logd/LogStatistics.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include #include @@ -824,8 +825,10 @@ uid_t pidToUid(pid_t pid) { FILE* fp = fopen(buffer, "r"); if (fp) { while (fgets(buffer, sizeof(buffer), fp)) { - int uid; - if (sscanf(buffer, "Uid: %d", &uid) == 1) { + int uid = AID_LOGD; + char space = 0; + if ((sscanf(buffer, "Uid: %d%c", &uid, &space) == 2) && + isspace(space)) { fclose(fp); return uid; } @@ -834,12 +837,35 @@ 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 945fc0ab6..8808aac70 100644 --- a/logd/LogStatistics.h +++ b/logd/LogStatistics.h @@ -306,10 +306,6 @@ 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; @@ -389,6 +385,13 @@ 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()), @@ -785,6 +788,7 @@ 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 fa9f39895..4dcd3e7f9 100644 --- a/logd/LogUtils.h +++ b/logd/LogUtils.h @@ -38,6 +38,8 @@ 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);