summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPaul Elliott2017-11-07 05:11:41 -0600
committerMark Salyzyn2017-11-07 11:57:26 -0600
commitc6ed8f39d3e4c45e83923dac93c1388526c99a3d (patch)
tree25eabb8701a885c2ad8912beca03848f00da3656 /logd/LogListener.cpp
parentebcfa449375fb809b266383d0036a7be3ecdac01 (diff)
downloadplatform-system-core-c6ed8f39d3e4c45e83923dac93c1388526c99a3d.tar.gz
platform-system-core-c6ed8f39d3e4c45e83923dac93c1388526c99a3d.tar.xz
platform-system-core-c6ed8f39d3e4c45e83923dac93c1388526c99a3d.zip
Buffer overrun in __android_log_is_loggable() fix
Fix for buffer overrun when a tag that is too big is sent to logd. Buffer supplied is precisely the right size for max message length however strlen will be run on the buffer, so need to ensure null terminator, otherwise any strlen will go off the end of the buffer. Also converted LogBuffer::Log() over to use the safer strnlen in the case where it is measuring the buffer (and converted over to using __android_log_is_loggable_len()) Signed-off-by: Paul Elliott <paul.elliott@arm.com> Test: liblog.android_log_buf_print__maxtag Change-Id: I3cb8b25af55943fb0f4658657560eb2300f52961
Diffstat (limited to 'logd/LogListener.cpp')
-rw-r--r--logd/LogListener.cpp10
1 files changed, 7 insertions, 3 deletions
diff --git a/logd/LogListener.cpp b/logd/LogListener.cpp
index d2df68eef..fcf2cd8a1 100644
--- a/logd/LogListener.cpp
+++ b/logd/LogListener.cpp
@@ -43,9 +43,10 @@ bool LogListener::onDataAvailable(SocketClient* cli) {
43 name_set = true; 43 name_set = true;
44 } 44 }
45 45
46 // + 1 to ensure null terminator if MAX_PAYLOAD buffer is received
46 char buffer[sizeof_log_id_t + sizeof(uint16_t) + sizeof(log_time) + 47 char buffer[sizeof_log_id_t + sizeof(uint16_t) + sizeof(log_time) +
47 LOGGER_ENTRY_MAX_PAYLOAD]; 48 LOGGER_ENTRY_MAX_PAYLOAD + 1];
48 struct iovec iov = { buffer, sizeof(buffer) }; 49 struct iovec iov = { buffer, sizeof(buffer) - 1 };
49 50
50 alignas(4) char control[CMSG_SPACE(sizeof(struct ucred))]; 51 alignas(4) char control[CMSG_SPACE(sizeof(struct ucred))];
51 struct msghdr hdr = { 52 struct msghdr hdr = {
@@ -55,13 +56,16 @@ bool LogListener::onDataAvailable(SocketClient* cli) {
55 int socket = cli->getSocket(); 56 int socket = cli->getSocket();
56 57
57 // To clear the entire buffer is secure/safe, but this contributes to 1.68% 58 // To clear the entire buffer is secure/safe, but this contributes to 1.68%
58 // overhead under logging load. We are safe because we check counts. 59 // overhead under logging load. We are safe because we check counts, but
60 // still need to clear null terminator
59 // memset(buffer, 0, sizeof(buffer)); 61 // memset(buffer, 0, sizeof(buffer));
60 ssize_t n = recvmsg(socket, &hdr, 0); 62 ssize_t n = recvmsg(socket, &hdr, 0);
61 if (n <= (ssize_t)(sizeof(android_log_header_t))) { 63 if (n <= (ssize_t)(sizeof(android_log_header_t))) {
62 return false; 64 return false;
63 } 65 }
64 66
67 buffer[n] = 0;
68
65 struct ucred* cred = NULL; 69 struct ucred* cred = NULL;
66 70
67 struct cmsghdr* cmsg = CMSG_FIRSTHDR(&hdr); 71 struct cmsghdr* cmsg = CMSG_FIRSTHDR(&hdr);