ACPR/android_system_core
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "Buffer overrun in __android_log_is_loggable() fix"

Browse source
This commit is contained in:
Mark Salyzyn 2017-11-08 16:06:38 +00:00 • committed by Gerrit Code Review
commit 35fc00124b
2 changed files with 14 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
logd/LogBuffer.cpp
View file

@ -212,13 +212,19 @@ int LogBuffer::log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid,
if (log_id != LOG_ID_SECURITY) { if (log_id != LOG_ID_SECURITY) {
int prio = ANDROID_LOG_INFO; int prio = ANDROID_LOG_INFO;
const char* tag = nullptr; const char* tag = nullptr;
size_t tag_len = 0;
if (log_id == LOG_ID_EVENTS) { if (log_id == LOG_ID_EVENTS) {
tag = tagToName(elem->getTag()); tag = tagToName(elem->getTag());
if (tag) {
tag_len = strlen(tag);
}
} else { } else {
prio = *msg; prio = *msg;
tag = msg + 1; tag = msg + 1;
tag_len = strnlen(tag, len - 1);
} }
if (!__android_log_is_loggable(prio, tag, ANDROID_LOG_VERBOSE)) { if (!__android_log_is_loggable_len(prio, tag, tag_len,
ANDROID_LOG_VERBOSE)) {
// Log traffic received to total // Log traffic received to total
wrlock(); wrlock();
stats.addTotal(elem); stats.addTotal(elem);

10
logd/LogListener.cpp
View file

@ -43,9 +43,10 @@ bool LogListener::onDataAvailable(SocketClient* cli) {
name_set = true; name_set = true;
} }
// + 1 to ensure null terminator if MAX_PAYLOAD buffer is received
char buffer[sizeof_log_id_t + sizeof(uint16_t) + sizeof(log_time) + char buffer[sizeof_log_id_t + sizeof(uint16_t) + sizeof(log_time) +
LOGGER_ENTRY_MAX_PAYLOAD]; LOGGER_ENTRY_MAX_PAYLOAD + 1];
struct iovec iov = { buffer, sizeof(buffer) }; struct iovec iov = { buffer, sizeof(buffer) - 1 };
alignas(4) char control[CMSG_SPACE(sizeof(struct ucred))]; alignas(4) char control[CMSG_SPACE(sizeof(struct ucred))];
struct msghdr hdr = { struct msghdr hdr = {
@ -55,13 +56,16 @@ bool LogListener::onDataAvailable(SocketClient* cli) {
int socket = cli->getSocket(); int socket = cli->getSocket();
// To clear the entire buffer is secure/safe, but this contributes to 1.68% // To clear the entire buffer is secure/safe, but this contributes to 1.68%
// overhead under logging load. We are safe because we check counts. // overhead under logging load. We are safe because we check counts, but
// still need to clear null terminator
// memset(buffer, 0, sizeof(buffer)); // memset(buffer, 0, sizeof(buffer));
ssize_t n = recvmsg(socket, &hdr, 0); ssize_t n = recvmsg(socket, &hdr, 0);
if (n <= (ssize_t)(sizeof(android_log_header_t))) { if (n <= (ssize_t)(sizeof(android_log_header_t))) {
return false; return false;
} }
buffer[n] = 0;
struct ucred* cred = NULL; struct ucred* cred = NULL;
struct cmsghdr* cmsg = CMSG_FIRSTHDR(&hdr); struct cmsghdr* cmsg = CMSG_FIRSTHDR(&hdr);