Skip to content
This repository has been archived by the owner on Nov 8, 2023. It is now read-only.

Commit

Permalink
logd: serialize accesses to stats helpers
Browse files Browse the repository at this point in the history
Quick low-risk to resolve possible hash table corruption.
Resolved an unlikely path memory leak.

ToDo: replace lock with nested lock so no lock
      helpers are required.

Bug: 22068332
Change-Id: I303ab06608502c7d61d42f111a9c43366f184d0c
  • Loading branch information
Mark Salyzyn committed Jun 25, 2015
1 parent 1a3334f commit ed777e9
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 4 deletions.
13 changes: 11 additions & 2 deletions logd/LogAudit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@ int LogAudit::logPrint(const char *fmt, ...) {
++cp;
}
tid = pid;
logbuf->lock();
uid = logbuf->pidToUid(pid);
logbuf->unlock();
memmove(pidptr, cp, strlen(cp) + 1);
}

Expand Down Expand Up @@ -180,14 +182,20 @@ int LogAudit::logPrint(const char *fmt, ...) {
static const char comm_str[] = " comm=\"";
const char *comm = strstr(str, comm_str);
const char *estr = str + strlen(str);
char *commfree = NULL;
if (comm) {
estr = comm;
comm += sizeof(comm_str) - 1;
} else if (pid == getpid()) {
pid = tid;
comm = "auditd";
} else if (!(comm = logbuf->pidToName(pid))) {
comm = "unknown";
} else {
logbuf->lock();
comm = commfree = logbuf->pidToName(pid);
logbuf->unlock();
if (!comm) {
comm = "unknown";
}
}

const char *ecomm = strchr(comm, '"');
Expand Down Expand Up @@ -218,6 +226,7 @@ int LogAudit::logPrint(const char *fmt, ...) {
}
}

free(commfree);
free(str);

if (notify) {
Expand Down
4 changes: 3 additions & 1 deletion logd/LogBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ class LogBuffer {
// *strp uses malloc, use free to release.
void formatPrune(char **strp) { mPrune.format(strp); }

// helper
// helper must be protected directly or implicitly by lock()/unlock()
char *pidToName(pid_t pid) { return stats.pidToName(pid); }
uid_t pidToUid(pid_t pid) { return stats.pidToUid(pid); }
char *uidToName(uid_t uid) { return stats.uidToName(uid); }
void lock() { pthread_mutex_lock(&mLogElementsLock); }
void unlock() { pthread_mutex_unlock(&mLogElementsLock); }

private:
void maybePrune(log_id_t id);
Expand Down
4 changes: 4 additions & 0 deletions logd/LogBufferElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,17 @@ size_t LogBufferElement::populateDroppedMessage(char *&buffer,
}

static const char format_uid[] = "uid=%u%s%s expire %u line%s";
parent->lock();
char *name = parent->uidToName(mUid);
parent->unlock();
char *commName = android::tidToName(mTid);
if (!commName && (mTid != mPid)) {
commName = android::tidToName(mPid);
}
if (!commName) {
parent->lock();
commName = parent->pidToName(mPid);
parent->unlock();
}
size_t len = name ? strlen(name) : 0;
if (len && commName && !strncmp(name, commName, len)) {
Expand Down
2 changes: 1 addition & 1 deletion logd/LogStatistics.h
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ class LogStatistics {
// *strp = malloc, balance with free
void format(char **strp, uid_t uid, unsigned int logMask);

// helper
// helper (must be locked directly or implicitly by mLogElementsLock)
char *pidToName(pid_t pid);
uid_t pidToUid(pid_t pid);
char *uidToName(uid_t uid);
Expand Down

0 comments on commit ed777e9

Please sign in to comment.