logging: update from upstream
Followed instructions from go/nnapi-dep-instructions.
No manual changes / interventions have been made.
$ git merge cros/upstream/master --no-ff
$ repo upload --cbr . --no-verify
BUG=b:177048246
TEST=cq passes
Exempt-From-Owner-Approval: This is a forked repo
Change-Id: Idbbd2d712b528176662694cebf869c0382587822
diff --git a/OWNERS b/OWNERS
index 096c4c5..7529cb9 100644
--- a/OWNERS
+++ b/OWNERS
@@ -1,2 +1 @@
-enh@google.com
-tomcherry@google.com
+include platform/system/core:/janitors/OWNERS
diff --git a/TEST_MAPPING b/TEST_MAPPING
new file mode 100644
index 0000000..5e72035
--- /dev/null
+++ b/TEST_MAPPING
@@ -0,0 +1,18 @@
+{
+ "presubmit": [
+ {
+ "name": "CtsLiblogTestCases"
+ },
+ {
+ "name": "CtsLogdTestCases"
+ },
+ {
+ "name": "liblog-host-test",
+ "host": true
+ },
+ {
+ "name": "logd-unit-tests",
+ "host": true
+ }
+ ]
+}
diff --git a/liblog/Android.bp b/liblog/Android.bp
index 8f15541..32bdde0 100644
--- a/liblog/Android.bp
+++ b/liblog/Android.bp
@@ -36,7 +36,9 @@
name: "liblog_headers",
host_supported: true,
vendor_available: true,
+ product_available: true,
ramdisk_available: true,
+ vendor_ramdisk_available: true,
recovery_available: true,
apex_available: [
"//apex_available:platform",
@@ -58,6 +60,9 @@
vendor: {
override_export_include_dirs: ["include_vndk"],
},
+ product: {
+ override_export_include_dirs: ["include_vndk"],
+ },
},
}
@@ -67,8 +72,10 @@
name: "liblog",
host_supported: true,
ramdisk_available: true,
+ vendor_ramdisk_available: true,
recovery_available: true,
native_bridge_supported: true,
+ llndk_stubs: "liblog.llndk",
srcs: liblog_sources,
target: {
@@ -131,6 +138,10 @@
"com.android.runtime",
// DO NOT add more apex names here
],
+ pgo: {
+ sampling: true,
+ profile_file: "logging/liblog.profdata",
+ },
}
ndk_headers {
@@ -149,7 +160,7 @@
}
llndk_library {
- name: "liblog",
+ name: "liblog.llndk",
native_bridge_supported: true,
symbol_file: "liblog.map.txt",
export_include_dirs: ["include_vndk"],
diff --git a/liblog/logd_writer.cpp b/liblog/logd_writer.cpp
index f5d19ca..3d5cee6 100644
--- a/liblog/logd_writer.cpp
+++ b/liblog/logd_writer.cpp
@@ -38,50 +38,82 @@
#include "logger.h"
#include "uio.h"
-static atomic_int logd_socket;
-
-// Note that it is safe to call connect() multiple times on DGRAM Unix domain sockets, so this
-// function is used to reconnect to logd without requiring a new socket.
-static void LogdConnect() {
- sockaddr_un un = {};
- un.sun_family = AF_UNIX;
- strcpy(un.sun_path, "/dev/socket/logdw");
- TEMP_FAILURE_RETRY(connect(logd_socket, reinterpret_cast<sockaddr*>(&un), sizeof(sockaddr_un)));
-}
-
-// logd_socket should only be opened once. If we see that logd_socket is uninitialized, we create a
-// new socket and attempt to exchange it into the atomic logd_socket. If the compare/exchange was
-// successful, then that will be the socket used for the duration of the program, otherwise a
-// different thread has already opened and written the socket to the atomic, so close the new socket
-// and return.
-static void GetSocket() {
- if (logd_socket != 0) {
- return;
+class LogdSocket {
+ public:
+ static LogdSocket& BlockingSocket() {
+ static LogdSocket logd_socket(true);
+ return logd_socket;
+ }
+ static LogdSocket& NonBlockingSocket() {
+ static LogdSocket logd_socket(false);
+ return logd_socket;
}
- int new_socket = TEMP_FAILURE_RETRY(socket(PF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0));
- if (new_socket <= 0) {
- return;
+ void Reconnect() { LogdConnect(sock_); }
+
+ // Zygote uses this to clean up open FD's after fork() and before specialization. It is single
+ // threaded at this point and therefore this function is explicitly not thread safe. It sets
+ // sock_ to kUninitialized, so future logs will be safely initialized whenever they happen.
+ void Close() {
+ if (sock_ != kUninitialized) {
+ close(sock_);
+ }
+ sock_ = kUninitialized;
}
- int uninitialized_value = 0;
- if (!logd_socket.compare_exchange_strong(uninitialized_value, new_socket)) {
- close(new_socket);
- return;
+ int sock() {
+ GetSocket();
+ return sock_;
}
- LogdConnect();
-}
+ private:
+ LogdSocket(bool blocking) : blocking_(blocking) {}
-// This is the one exception to the above. Zygote uses this to clean up open FD's after fork() and
-// before specialization. It is single threaded at this point and therefore this function is
-// explicitly not thread safe. It sets logd_socket to 0, so future logs will be safely initialized
-// whenever they happen.
+ // Note that it is safe to call connect() multiple times on DGRAM Unix domain sockets, so this
+ // function is used to reconnect to logd without requiring a new socket.
+ static void LogdConnect(int sock) {
+ sockaddr_un un = {};
+ un.sun_family = AF_UNIX;
+ strcpy(un.sun_path, "/dev/socket/logdw");
+ TEMP_FAILURE_RETRY(connect(sock, reinterpret_cast<sockaddr*>(&un), sizeof(sockaddr_un)));
+ }
+
+ // sock_ should only be opened once. If we see that sock_ is uninitialized, we
+ // create a new socket and attempt to exchange it into the atomic sock_. If the
+ // compare/exchange was successful, then that will be the socket used for the duration of the
+ // program, otherwise a different thread has already opened and written the socket to the atomic,
+ // so close the new socket and return.
+ void GetSocket() {
+ if (sock_ != kUninitialized) {
+ return;
+ }
+
+ int flags = SOCK_DGRAM | SOCK_CLOEXEC;
+ if (!blocking_) {
+ flags |= SOCK_NONBLOCK;
+ }
+ int new_socket = TEMP_FAILURE_RETRY(socket(PF_UNIX, flags, 0));
+ if (new_socket < 0) {
+ return;
+ }
+
+ LogdConnect(new_socket);
+
+ int uninitialized_value = kUninitialized;
+ if (!sock_.compare_exchange_strong(uninitialized_value, new_socket)) {
+ close(new_socket);
+ return;
+ }
+ }
+
+ static const int kUninitialized = -1;
+ atomic_int sock_ = kUninitialized;
+ bool blocking_;
+};
+
void LogdClose() {
- if (logd_socket > 0) {
- close(logd_socket);
- }
- logd_socket = 0;
+ LogdSocket::BlockingSocket().Close();
+ LogdSocket::NonBlockingSocket().Close();
}
int LogdWrite(log_id_t logId, struct timespec* ts, struct iovec* vec, size_t nr) {
@@ -90,10 +122,12 @@
struct iovec newVec[nr + headerLength];
android_log_header_t header;
size_t i, payloadSize;
+ static atomic_int dropped;
- GetSocket();
+ LogdSocket& logd_socket =
+ logId == LOG_ID_SECURITY ? LogdSocket::BlockingSocket() : LogdSocket::NonBlockingSocket();
- if (logd_socket <= 0) {
+ if (logd_socket.sock() < 0) {
return -EBADF;
}
@@ -107,7 +141,6 @@
return 0;
}
- header.id = logId;
header.tid = gettid();
header.realtime.tv_sec = ts->tv_sec;
header.realtime.tv_nsec = ts->tv_nsec;
@@ -115,6 +148,27 @@
newVec[0].iov_base = (unsigned char*)&header;
newVec[0].iov_len = sizeof(header);
+ int32_t snapshot = atomic_exchange_explicit(&dropped, 0, memory_order_relaxed);
+ if (snapshot && __android_log_is_loggable_len(ANDROID_LOG_INFO, "liblog", strlen("liblog"),
+ ANDROID_LOG_VERBOSE)) {
+ android_log_event_int_t buffer;
+
+ header.id = LOG_ID_EVENTS;
+ buffer.header.tag = LIBLOG_LOG_TAG;
+ buffer.payload.type = EVENT_TYPE_INT;
+ buffer.payload.data = snapshot;
+
+ newVec[headerLength].iov_base = &buffer;
+ newVec[headerLength].iov_len = sizeof(buffer);
+
+ ret = TEMP_FAILURE_RETRY(writev(logd_socket.sock(), newVec, 2));
+ if (ret != (ssize_t)(sizeof(header) + sizeof(buffer))) {
+ atomic_fetch_add_explicit(&dropped, snapshot, memory_order_relaxed);
+ }
+ }
+
+ header.id = logId;
+
for (payloadSize = 0, i = headerLength; i < nr + headerLength; i++) {
newVec[i].iov_base = vec[i - headerLength].iov_base;
payloadSize += newVec[i].iov_len = vec[i - headerLength].iov_len;
@@ -128,16 +182,24 @@
}
}
- ret = TEMP_FAILURE_RETRY(writev(logd_socket, newVec, i));
- if (ret < 0) {
- LogdConnect();
+ // EAGAIN occurs if logd is overloaded, other errors indicate that something went wrong with
+ // the connection, so we reset it and try again.
+ ret = TEMP_FAILURE_RETRY(writev(logd_socket.sock(), newVec, i));
+ if (ret < 0 && errno != EAGAIN) {
+ logd_socket.Reconnect();
- ret = TEMP_FAILURE_RETRY(writev(logd_socket, newVec, i));
+ ret = TEMP_FAILURE_RETRY(writev(logd_socket.sock(), newVec, i));
}
if (ret < 0) {
ret = -errno;
}
+ if (ret > (ssize_t)sizeof(header)) {
+ ret -= sizeof(header);
+ } else if (ret < 0) {
+ atomic_fetch_add_explicit(&dropped, 1, memory_order_relaxed);
+ }
+
return ret;
}
diff --git a/liblog/pmsg_writer.cpp b/liblog/pmsg_writer.cpp
index 8e676bd..037a72c 100644
--- a/liblog/pmsg_writer.cpp
+++ b/liblog/pmsg_writer.cpp
@@ -31,24 +31,32 @@
static atomic_int pmsg_fd;
-// pmsg_fd should only beopened once. If we see that pmsg_fd is uninitialized, we open "/dev/pmsg0"
-// then attempt to compare/exchange it into pmsg_fd. If the compare/exchange was successful, then
-// that will be the fd used for the duration of the program, otherwise a different thread has
-// already opened and written the fd to the atomic, so close the new fd and return.
static void GetPmsgFd() {
+ // Note if open() fails and returns -1, that value is stored into pmsg_fd as an indication that
+ // pmsg is not available and open() should not be retried.
if (pmsg_fd != 0) {
return;
}
int new_fd = TEMP_FAILURE_RETRY(open("/dev/pmsg0", O_WRONLY | O_CLOEXEC));
- if (new_fd <= 0) {
- return;
+
+ // Unlikely that new_fd is 0, but that is synonymous with our uninitialized value, and we'd prefer
+ // STDIN_FILENO != stdin, so we call open() to get a new fd value in this case.
+ if (new_fd == 0) {
+ new_fd = TEMP_FAILURE_RETRY(open("/dev/pmsg0", O_WRONLY | O_CLOEXEC));
+ close(0);
}
+ // pmsg_fd should only be opened once. If we see that pmsg_fd is uninitialized, we open
+ // "/dev/pmsg0" then attempt to compare/exchange it into pmsg_fd. If the compare/exchange was
+ // successful, then that will be the fd used for the duration of the program, otherwise a
+ // different thread has already opened and written the fd to the atomic, so close the new fd and
+ // return.
int uninitialized_value = 0;
if (!pmsg_fd.compare_exchange_strong(uninitialized_value, new_fd)) {
- close(new_fd);
- return;
+ if (new_fd != -1) {
+ close(new_fd);
+ }
}
}
diff --git a/liblog/tests/Android.bp b/liblog/tests/Android.bp
index 171aafd..7524750 100644
--- a/liblog/tests/Android.bp
+++ b/liblog/tests/Android.bp
@@ -99,6 +99,7 @@
"cts",
"device-tests",
],
+ test_config: "device_test_config.xml",
}
cc_test_host {
@@ -111,4 +112,7 @@
"liblog_global_state.cpp",
],
isolated: true,
+ test_suites: [
+ "general-tests",
+ ],
}
diff --git a/liblog/tests/AndroidTest.xml b/liblog/tests/device_test_config.xml
similarity index 100%
rename from liblog/tests/AndroidTest.xml
rename to liblog/tests/device_test_config.xml
diff --git a/liblog/tests/liblog_benchmark.cpp b/liblog/tests/liblog_benchmark.cpp
index d2f12d6..5f1750b 100644
--- a/liblog/tests/liblog_benchmark.cpp
+++ b/liblog/tests/liblog_benchmark.cpp
@@ -594,8 +594,8 @@
// In system/core/logcat/event.logtags:
// # These are used for testing, do not modify without updating
// # tests/framework-tests/src/android/util/EventLogFunctionalTest.java.
- // # system/core/liblog/tests/liblog_benchmark.cpp
- // # system/core/liblog/tests/liblog_test.cpp
+ // # system/logging/liblog/tests/liblog_benchmark.cpp
+ // # system/logging/liblog/tests/liblog_test.cpp
// 42 answer (to life the universe etc|3)
__android_log_btwrite(42, EVENT_TYPE_LONG, &i, sizeof(i));
state.PauseTiming();
diff --git a/logcat/event.logtags b/logcat/event.logtags
index 93c3d6d..210cac7 100644
--- a/logcat/event.logtags
+++ b/logcat/event.logtags
@@ -37,8 +37,8 @@
# These are used for testing, do not modify without updating
# tests/framework-tests/src/android/util/EventLogFunctionalTest.java.
-# system/core/liblog/tests/liblog_benchmark.cpp
-# system/core/liblog/tests/liblog_test.cpp
+# system/logging/liblog/tests/liblog_benchmark.cpp
+# system/logging/liblog/tests/liblog_test.cpp
42 answer (to life the universe etc|3)
314 pi
2718 e
diff --git a/logcat/logcat.cpp b/logcat/logcat.cpp
index 6b7e016..67ee676 100644
--- a/logcat/logcat.cpp
+++ b/logcat/logcat.cpp
@@ -101,6 +101,7 @@
bool print_it_anyways_ = false;
// For PrintDividers()
+ bool print_dividers_ = false;
log_id_t last_printed_id_ = LOG_ID_MAX;
bool printed_start_[LOG_ID_MAX] = {};
@@ -215,6 +216,8 @@
print_count_ += match;
if (match || print_it_anyways_) {
+ PrintDividers(buf->id(), print_dividers_);
+
bytesWritten = android_log_printLogLine(logformat_.get(), output_fd_.get(), &entry);
if (bytesWritten < 0) {
@@ -231,7 +234,7 @@
}
void Logcat::PrintDividers(log_id_t log_id, bool print_dividers) {
- if (log_id == last_printed_id_ || print_binary_) {
+ if (log_id == last_printed_id_) {
return;
}
if (!printed_start_[log_id] || print_dividers) {
@@ -530,7 +533,6 @@
bool getLogSize = false;
bool getPruneList = false;
bool printStatistics = false;
- bool printDividers = false;
unsigned long setLogSize = 0;
const char* setPruneList = nullptr;
const char* setId = nullptr;
@@ -696,7 +698,7 @@
break;
case 'D':
- printDividers = true;
+ print_dividers_ = true;
break;
case 'e':
@@ -1193,8 +1195,6 @@
continue;
}
- PrintDividers(log_msg.id(), printDividers);
-
if (print_binary_) {
if (!WriteFully(output_fd_, &log_msg, log_msg.len())) {
error(EXIT_FAILURE, errno, "Failed to write to output fd");
diff --git a/logd/Android.bp b/logd/Android.bp
index 829c95f..63bd505 100644
--- a/logd/Android.bp
+++ b/logd/Android.bp
@@ -91,6 +91,7 @@
"LogAudit.cpp",
"LogKlog.cpp",
"libaudit.cpp",
+ "PkgIds.cpp",
],
static_libs: [
@@ -172,6 +173,9 @@
name: "logd-unit-tests",
host_supported: true,
defaults: ["logd-unit-test-defaults"],
+ test_suites: [
+ "general-tests",
+ ],
}
cc_test {
@@ -189,6 +193,7 @@
"cts",
"device-tests",
],
+ test_config: "device_test_config.xml",
}
cc_binary {
diff --git a/logd/ChattyLogBufferTest.cpp b/logd/ChattyLogBufferTest.cpp
index e273efe..dd9375d 100644
--- a/logd/ChattyLogBufferTest.cpp
+++ b/logd/ChattyLogBufferTest.cpp
@@ -59,16 +59,6 @@
FixupMessages(&log_messages);
LogMessages(log_messages);
- std::vector<LogMessage> read_log_messages;
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, nullptr));
-
- std::unique_ptr<FlushToState> flush_to_state =
- log_buffer_->CreateFlushToState(1, kLogMaskAll);
- EXPECT_TRUE(log_buffer_->FlushTo(test_writer.get(), *flush_to_state, nullptr));
- }
-
std::vector<LogMessage> expected_log_messages = {
make_message(0, "test_tag", "duplicate"),
make_message(1, "test_tag", "duplicate"),
@@ -92,7 +82,8 @@
make_message(300, "test_tag", "duplicate"),
};
FixupMessages(&expected_log_messages);
- CompareLogMessages(expected_log_messages, read_log_messages);
+ auto flush_result = FlushMessages();
+ CompareLogMessages(expected_log_messages, flush_result.messages);
};
TEST_P(ChattyLogBufferTest, deduplication_overflow) {
@@ -121,15 +112,6 @@
FixupMessages(&log_messages);
LogMessages(log_messages);
- std::vector<LogMessage> read_log_messages;
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, nullptr));
- std::unique_ptr<FlushToState> flush_to_state =
- log_buffer_->CreateFlushToState(1, kLogMaskAll);
- EXPECT_TRUE(log_buffer_->FlushTo(test_writer.get(), *flush_to_state, nullptr));
- }
-
std::vector<LogMessage> expected_log_messages = {
make_message(0, "test_tag", "normal"),
make_message(1, "test_tag", "duplicate"),
@@ -141,7 +123,8 @@
make_message(expired_per_chatty_message + 4, "test_tag", "normal"),
};
FixupMessages(&expected_log_messages);
- CompareLogMessages(expected_log_messages, read_log_messages);
+ auto flush_result = FlushMessages();
+ CompareLogMessages(expected_log_messages, flush_result.messages);
}
TEST_P(ChattyLogBufferTest, deduplication_liblog) {
@@ -181,15 +164,6 @@
FixupMessages(&log_messages);
LogMessages(log_messages);
- std::vector<LogMessage> read_log_messages;
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, nullptr));
- std::unique_ptr<FlushToState> flush_to_state =
- log_buffer_->CreateFlushToState(1, kLogMaskAll);
- EXPECT_TRUE(log_buffer_->FlushTo(test_writer.get(), *flush_to_state, nullptr));
- }
-
std::vector<LogMessage> expected_log_messages = {
make_message(0, 1234, 1),
make_message(1, LIBLOG_LOG_TAG, 3),
@@ -212,7 +186,8 @@
make_message(20, 1234, 227),
};
FixupMessages(&expected_log_messages);
- CompareLogMessages(expected_log_messages, read_log_messages);
+ auto flush_result = FlushMessages();
+ CompareLogMessages(expected_log_messages, flush_result.messages);
};
TEST_P(ChattyLogBufferTest, no_leading_chatty_simple) {
@@ -267,21 +242,7 @@
FixupMessages(&expected_log_messages);
// clang-format on
- std::vector<LogMessage> read_log_messages;
- bool released = false;
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, &released));
- std::unique_ptr<LogReaderThread> log_reader(
- new LogReaderThread(log_buffer_.get(), &reader_list_, std::move(test_writer), true,
- 0, ~0, 1, {}, 2, {}));
- reader_list_.reader_threads().emplace_back(std::move(log_reader));
- }
-
- while (!released) {
- usleep(5000);
- }
-
+ auto read_log_messages = ReadLogMessagesNonBlockingThread({.pid = 1, .sequence = 2});
CompareLogMessages(expected_log_messages, read_log_messages);
}
@@ -327,21 +288,7 @@
FixupMessages(&expected_log_messages);
// clang-format on
- std::vector<LogMessage> read_log_messages;
- bool released = false;
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, &released));
- std::unique_ptr<LogReaderThread> log_reader(
- new LogReaderThread(log_buffer_.get(), &reader_list_, std::move(test_writer), true,
- 3, ~0, 0, {}, 1, {}));
- reader_list_.reader_threads().emplace_back(std::move(log_reader));
- }
-
- while (!released) {
- usleep(5000);
- }
-
+ auto read_log_messages = ReadLogMessagesNonBlockingThread({.tail = 3});
CompareLogMessages(expected_log_messages, read_log_messages);
}
diff --git a/logd/LogBufferTest.cpp b/logd/LogBufferTest.cpp
index cb9f428..e881e76 100644
--- a/logd/LogBufferTest.cpp
+++ b/logd/LogBufferTest.cpp
@@ -189,16 +189,9 @@
FixupMessages(&log_messages);
LogMessages(log_messages);
- std::vector<LogMessage> read_log_messages;
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, nullptr));
- std::unique_ptr<FlushToState> flush_to_state =
- log_buffer_->CreateFlushToState(1, kLogMaskAll);
- EXPECT_TRUE(log_buffer_->FlushTo(test_writer.get(), *flush_to_state, nullptr));
- EXPECT_EQ(2ULL, flush_to_state->start());
- }
- CompareLogMessages(log_messages, read_log_messages);
+ auto flush_result = FlushMessages();
+ EXPECT_EQ(2ULL, flush_result.next_sequence);
+ CompareLogMessages(log_messages, flush_result.messages);
}
TEST_P(LogBufferTest, smoke_with_reader_thread) {
@@ -227,25 +220,7 @@
FixupMessages(&log_messages);
LogMessages(log_messages);
- std::vector<LogMessage> read_log_messages;
- bool released = false;
-
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, &released));
- std::unique_ptr<LogReaderThread> log_reader(
- new LogReaderThread(log_buffer_.get(), &reader_list_, std::move(test_writer), true,
- 0, kLogMaskAll, 0, {}, 1, {}));
- reader_list_.reader_threads().emplace_back(std::move(log_reader));
- }
-
- while (!released) {
- usleep(5000);
- }
- {
- auto lock = std::lock_guard{logd_lock};
- EXPECT_EQ(0U, reader_list_.reader_threads().size());
- }
+ auto read_log_messages = ReadLogMessagesNonBlockingThread({});
CompareLogMessages(log_messages, read_log_messages);
}
@@ -293,33 +268,20 @@
return {entry, message};
}
-TEST_P(LogBufferTest, random_messages) {
+std::vector<LogMessage> GenerateRandomLogMessages(size_t count) {
srand(1);
std::vector<LogMessage> log_messages;
- for (size_t i = 0; i < 1000; ++i) {
+ for (size_t i = 0; i < count; ++i) {
log_messages.emplace_back(GenerateRandomLogMessage(i));
}
+ return log_messages;
+}
+
+TEST_P(LogBufferTest, random_messages) {
+ auto log_messages = GenerateRandomLogMessages(1000);
LogMessages(log_messages);
- std::vector<LogMessage> read_log_messages;
- bool released = false;
-
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, &released));
- std::unique_ptr<LogReaderThread> log_reader(
- new LogReaderThread(log_buffer_.get(), &reader_list_, std::move(test_writer), true,
- 0, kLogMaskAll, 0, {}, 1, {}));
- reader_list_.reader_threads().emplace_back(std::move(log_reader));
- }
-
- while (!released) {
- usleep(5000);
- }
- {
- auto lock = std::lock_guard{logd_lock};
- EXPECT_EQ(0U, reader_list_.reader_threads().size());
- }
+ auto read_log_messages = ReadLogMessagesNonBlockingThread({});
CompareLogMessages(log_messages, read_log_messages);
}
@@ -335,26 +297,8 @@
FixupMessages(&log_messages);
LogMessages(log_messages);
- std::vector<LogMessage> read_log_messages;
- bool released = false;
-
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, &released));
- std::unique_ptr<LogReaderThread> log_reader(
- new LogReaderThread(log_buffer_.get(), &reader_list_, std::move(test_writer), true,
- 0, kLogMaskAll, 0, {}, 3, {}));
- reader_list_.reader_threads().emplace_back(std::move(log_reader));
- }
-
- while (!released) {
- usleep(5000);
- }
- {
- auto lock = std::lock_guard{logd_lock};
- EXPECT_EQ(0U, reader_list_.reader_threads().size());
- }
std::vector<LogMessage> expected_log_messages = {log_messages.back()};
+ auto read_log_messages = ReadLogMessagesNonBlockingThread({.sequence = 3});
CompareLogMessages(expected_log_messages, read_log_messages);
}
@@ -371,18 +315,8 @@
FixupMessages(&log_messages);
LogMessages(log_messages);
- std::vector<LogMessage> read_log_messages;
- bool released = false;
-
// Connect a blocking reader.
- {
- auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, &released));
- std::unique_ptr<LogReaderThread> log_reader(
- new LogReaderThread(log_buffer_.get(), &reader_list_, std::move(test_writer), false,
- 0, kLogMaskAll, 0, {}, 1, {}));
- reader_list_.reader_threads().emplace_back(std::move(log_reader));
- }
+ auto blocking_reader = TestReaderThread({.non_block = false}, *this);
// Wait up to 250ms for the reader to read the first 3 logs.
constexpr int kMaxRetryCount = 50;
@@ -421,37 +355,130 @@
}
ASSERT_LT(count, kMaxRetryCount);
- // Release the reader, wait for it to get the signal then check that it has been deleted.
- {
- auto lock = std::lock_guard{logd_lock};
- reader_list_.reader_threads().back()->Release();
- }
- while (!released) {
- usleep(5000);
- }
- {
- auto lock = std::lock_guard{logd_lock};
- EXPECT_EQ(0U, reader_list_.reader_threads().size());
- }
+ ReleaseAndJoinReaders();
// Check that we have read all 6 messages.
std::vector<LogMessage> expected_log_messages = log_messages;
expected_log_messages.insert(expected_log_messages.end(), after_clear_messages.begin(),
after_clear_messages.end());
- CompareLogMessages(expected_log_messages, read_log_messages);
+ CompareLogMessages(expected_log_messages, blocking_reader.read_log_messages());
- // Finally, call FlushTo and ensure that only the 3 logs after the clear remain in the buffer.
- std::vector<LogMessage> read_log_messages_after_clear;
- {
+ // Finally, Flush messages and ensure that only the 3 logs after the clear remain in the buffer.
+ auto flush_after_clear_result = FlushMessages();
+ EXPECT_EQ(7ULL, flush_after_clear_result.next_sequence);
+ CompareLogMessages(after_clear_messages, flush_after_clear_result.messages);
+}
+
+TEST_P(LogBufferTest, tail100_nonblocking_1000total) {
+ auto log_messages = GenerateRandomLogMessages(1000);
+ LogMessages(log_messages);
+
+ constexpr int kTailCount = 100;
+ std::vector<LogMessage> expected_log_messages{log_messages.end() - kTailCount,
+ log_messages.end()};
+ auto read_log_messages = ReadLogMessagesNonBlockingThread({.tail = kTailCount});
+ CompareLogMessages(expected_log_messages, read_log_messages);
+}
+
+TEST_P(LogBufferTest, tail100_blocking_1000total_then1000more) {
+ auto log_messages = GenerateRandomLogMessages(1000);
+ LogMessages(log_messages);
+
+ constexpr int kTailCount = 100;
+ auto blocking_reader = TestReaderThread({.non_block = false, .tail = kTailCount}, *this);
+
+ std::vector<LogMessage> expected_log_messages{log_messages.end() - kTailCount,
+ log_messages.end()};
+
+ // Wait for the reader to have read the messages.
+ int retry_count = 1s / 5000us;
+ while (retry_count--) {
+ usleep(5000);
auto lock = std::lock_guard{logd_lock};
- std::unique_ptr<LogWriter> test_writer(
- new TestWriter(&read_log_messages_after_clear, nullptr));
- std::unique_ptr<FlushToState> flush_to_state =
- log_buffer_->CreateFlushToState(1, kLogMaskAll);
- EXPECT_TRUE(log_buffer_->FlushTo(test_writer.get(), *flush_to_state, nullptr));
- EXPECT_EQ(7ULL, flush_to_state->start());
+ if (blocking_reader.read_log_messages().size() == expected_log_messages.size()) {
+ CompareLogMessages(expected_log_messages, blocking_reader.read_log_messages());
+ break;
+ }
}
- CompareLogMessages(after_clear_messages, read_log_messages_after_clear);
+ ASSERT_GT(retry_count, 0);
+
+ // Log more messages
+ log_messages = GenerateRandomLogMessages(1000);
+ LogMessages(log_messages);
+ expected_log_messages.insert(expected_log_messages.end(), log_messages.begin(),
+ log_messages.end());
+
+ // Wait for the reader to have read the new messages.
+ retry_count = 1s / 5000us;
+ while (retry_count--) {
+ usleep(5000);
+ auto lock = std::lock_guard{logd_lock};
+ if (blocking_reader.read_log_messages().size() == expected_log_messages.size()) {
+ CompareLogMessages(expected_log_messages, blocking_reader.read_log_messages());
+ break;
+ }
+ }
+ ASSERT_GT(retry_count, 0);
+
+ ReleaseAndJoinReaders();
+
+ // Final check that no extraneous logs were logged.
+ CompareLogMessages(expected_log_messages, blocking_reader.read_log_messages());
+}
+
+TEST_P(LogBufferTest, tail100_nonblocking_50total) {
+ auto log_messages = GenerateRandomLogMessages(50);
+ LogMessages(log_messages);
+
+ constexpr int kTailCount = 100;
+ auto read_log_messages = ReadLogMessagesNonBlockingThread({.tail = kTailCount});
+ CompareLogMessages(log_messages, read_log_messages);
+}
+
+TEST_P(LogBufferTest, tail100_blocking_50total_then1000more) {
+ auto log_messages = GenerateRandomLogMessages(50);
+ LogMessages(log_messages);
+
+ constexpr int kTailCount = 100;
+ auto blocking_reader = TestReaderThread({.non_block = false, .tail = kTailCount}, *this);
+
+ std::vector<LogMessage> expected_log_messages = log_messages;
+
+ // Wait for the reader to have read the messages.
+ int retry_count = 1s / 5000us;
+ while (retry_count--) {
+ usleep(5000);
+ auto lock = std::lock_guard{logd_lock};
+ if (blocking_reader.read_log_messages().size() == expected_log_messages.size()) {
+ CompareLogMessages(expected_log_messages, blocking_reader.read_log_messages());
+ break;
+ }
+ }
+ ASSERT_GT(retry_count, 0);
+
+ // Log more messages
+ log_messages = GenerateRandomLogMessages(1000);
+ LogMessages(log_messages);
+ expected_log_messages.insert(expected_log_messages.end(), log_messages.begin(),
+ log_messages.end());
+
+ // Wait for the reader to have read the new messages.
+ retry_count = 1s / 5000us;
+ while (retry_count--) {
+ usleep(5000);
+ auto lock = std::lock_guard{logd_lock};
+ if (blocking_reader.read_log_messages().size() == expected_log_messages.size()) {
+ CompareLogMessages(expected_log_messages, blocking_reader.read_log_messages());
+
+ break;
+ }
+ }
+ ASSERT_GT(retry_count, 0);
+
+ ReleaseAndJoinReaders();
+
+ // Final check that no extraneous logs were logged.
+ CompareLogMessages(expected_log_messages, blocking_reader.read_log_messages());
}
INSTANTIATE_TEST_CASE_P(LogBufferTests, LogBufferTest,
diff --git a/logd/LogBufferTest.h b/logd/LogBufferTest.h
index eeeb980..0d42bd3 100644
--- a/logd/LogBufferTest.h
+++ b/logd/LogBufferTest.h
@@ -16,12 +16,14 @@
#pragma once
+#include <chrono>
#include <string>
#include <vector>
#include <gtest/gtest.h>
#include "ChattyLogBuffer.h"
+#include "LogBuffer.h"
#include "LogReaderList.h"
#include "LogStatistics.h"
#include "LogTags.h"
@@ -29,6 +31,8 @@
#include "SerializedLogBuffer.h"
#include "SimpleLogBuffer.h"
+using namespace std::chrono_literals;
+
struct LogMessage {
logger_entry entry;
std::string message;
@@ -81,11 +85,95 @@
void LogMessages(const std::vector<LogMessage>& messages) {
for (auto& [entry, message, _] : messages) {
- log_buffer_->Log(static_cast<log_id_t>(entry.lid), log_time(entry.sec, entry.nsec),
- entry.uid, entry.pid, entry.tid, message.c_str(), message.size());
+ EXPECT_GT(log_buffer_->Log(static_cast<log_id_t>(entry.lid),
+ log_time(entry.sec, entry.nsec), entry.uid, entry.pid,
+ entry.tid, message.c_str(), message.size()),
+ 0);
}
}
+ struct FlushMessagesResult {
+ std::vector<LogMessage> messages;
+ uint64_t next_sequence;
+ };
+
+ FlushMessagesResult FlushMessages(uint64_t sequence = 1, LogMask log_mask = kLogMaskAll) {
+ std::vector<LogMessage> read_log_messages;
+ auto lock = std::lock_guard{logd_lock};
+ std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, nullptr));
+
+ auto flush_to_state = log_buffer_->CreateFlushToState(sequence, log_mask);
+ EXPECT_TRUE(log_buffer_->FlushTo(test_writer.get(), *flush_to_state, nullptr));
+ return {read_log_messages, flush_to_state->start()};
+ }
+
+ struct ReaderThreadParams {
+ bool non_block = true;
+ unsigned long tail = 0;
+ LogMask log_mask = kLogMaskAll;
+ pid_t pid = 0;
+ log_time start_time = {};
+ uint64_t sequence = 1;
+ std::chrono::steady_clock::time_point deadline = {};
+ };
+
+ class TestReaderThread {
+ public:
+ TestReaderThread(const ReaderThreadParams& params, LogBufferTest& test) : test_(test) {
+ auto lock = std::lock_guard{logd_lock};
+ std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages_, &released_));
+ std::unique_ptr<LogReaderThread> log_reader(new LogReaderThread(
+ test_.log_buffer_.get(), &test_.reader_list_, std::move(test_writer),
+ params.non_block, params.tail, params.log_mask, params.pid, params.start_time,
+ params.sequence, params.deadline));
+ test_.reader_list_.reader_threads().emplace_back(std::move(log_reader));
+ }
+
+ void WaitUntilReleased() {
+ while (!released_) {
+ usleep(5000);
+ }
+ }
+
+ std::vector<LogMessage> read_log_messages() const { return read_log_messages_; }
+
+ LogBufferTest& test_;
+ std::vector<LogMessage> read_log_messages_;
+ bool released_ = false;
+ };
+
+ std::vector<LogMessage> ReadLogMessagesNonBlockingThread(const ReaderThreadParams& params) {
+ EXPECT_TRUE(params.non_block)
+ << "params.non_block must be true for ReadLogMessagesNonBlockingThread()";
+
+ auto reader = TestReaderThread(params, *this);
+ reader.WaitUntilReleased();
+ auto lock = std::lock_guard{logd_lock};
+ EXPECT_EQ(0U, reader_list_.reader_threads().size());
+
+ return reader.read_log_messages();
+ }
+
+ void ReleaseAndJoinReaders() {
+ {
+ auto lock = std::lock_guard{logd_lock};
+ for (auto& reader : reader_list_.reader_threads()) {
+ reader->Release();
+ }
+ }
+
+ auto retries = 1s / 5000us;
+ while (retries--) {
+ usleep(5000);
+ auto lock = std::lock_guard{logd_lock};
+ if (reader_list_.reader_threads().size() == 0) {
+ return;
+ }
+ }
+
+ FAIL() << "ReleaseAndJoinReaders() timed out with reader threads still running";
+ }
+
LogReaderList reader_list_;
LogTags tags_;
PruneList prune_;
diff --git a/logd/LogPermissions.cpp b/logd/LogPermissions.cpp
index 3fd0ea1..63a6d3b 100644
--- a/logd/LogPermissions.cpp
+++ b/logd/LogPermissions.cpp
@@ -20,22 +20,12 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/socket.h>
+
+#include <vector>
#include <private/android_filesystem_config.h>
-// gets a list of supplementary group IDs associated with
-// the socket peer. This is implemented by opening
-// /proc/PID/status and look for the "Group:" line.
-//
-// This function introduces races especially since status
-// can change 'shape' while reading, the net result is err
-// on lack of permission.
-//
-// Race-free alternative is to introduce pairs of sockets
-// and threads for each command and reading, one each that
-// has open permissions, and one that has restricted
-// permissions.
-
static bool groupIsLog(char* buf) {
char* ptr;
static const char ws[] = " \n";
@@ -53,12 +43,19 @@
return false;
}
-bool clientHasLogCredentials(uid_t uid, gid_t gid, pid_t pid) {
- if ((uid == AID_ROOT) || (uid == AID_SYSTEM) || (uid == AID_LOG)) {
- return true;
- }
+static bool UserIsPrivileged(int id) {
+ return id == AID_ROOT || id == AID_SYSTEM || id == AID_LOG;
+}
- if ((gid == AID_ROOT) || (gid == AID_SYSTEM) || (gid == AID_LOG)) {
+// gets a list of supplementary group IDs associated with
+// the socket peer. This is implemented by opening
+// /proc/PID/status and look for the "Group:" line.
+//
+// This function introduces races especially since status
+// can change 'shape' while reading, the net result is err
+// on lack of permission.
+bool clientHasLogCredentials(uid_t uid, gid_t gid, pid_t pid) {
+ if (UserIsPrivileged(uid) || UserIsPrivileged(gid)) {
return true;
}
@@ -73,8 +70,7 @@
//
// Reading /proc/<pid>/status is rife with race conditions. All of /proc
- // suffers from this and its use should be minimized. However, we have no
- // choice.
+ // suffers from this and its use should be minimized.
//
// Notably the content from one 4KB page to the next 4KB page can be from a
// change in shape even if we are gracious enough to attempt to read
@@ -137,5 +133,42 @@
}
bool clientHasLogCredentials(SocketClient* cli) {
- return clientHasLogCredentials(cli->getUid(), cli->getGid(), cli->getPid());
+ if (UserIsPrivileged(cli->getUid()) || UserIsPrivileged(cli->getGid())) {
+ return true;
+ }
+
+ // Kernel version 4.13 added SO_PEERGROUPS to return the supplemental groups of a peer socket,
+ // so try that first then fallback to the above racy checking of /proc/<pid>/status if the
+ // kernel is too old. Per
+ // https://source.android.com/devices/architecture/kernel/android-common, the fallback can be
+ // removed no earlier than 2024.
+ auto supplemental_groups = std::vector<gid_t>(16, -1);
+ socklen_t groups_size = supplemental_groups.size() * sizeof(gid_t);
+
+ int result = getsockopt(cli->getSocket(), SOL_SOCKET, SO_PEERGROUPS, supplemental_groups.data(),
+ &groups_size);
+
+ if (result != 0) {
+ if (errno != ERANGE) {
+ return clientHasLogCredentials(cli->getUid(), cli->getGid(), cli->getPid());
+ }
+
+ supplemental_groups.resize(groups_size / sizeof(gid_t), -1);
+ result = getsockopt(cli->getSocket(), SOL_SOCKET, SO_PEERGROUPS, supplemental_groups.data(),
+ &groups_size);
+
+ // There is still some error after resizing supplemental_groups, fallback.
+ if (result != 0) {
+ return clientHasLogCredentials(cli->getUid(), cli->getGid(), cli->getPid());
+ }
+ }
+
+ supplemental_groups.resize(groups_size / sizeof(gid_t), -1);
+ for (const auto& gid : supplemental_groups) {
+ if (UserIsPrivileged(gid)) {
+ return true;
+ }
+ }
+
+ return false;
}
diff --git a/logd/PkgIds.cpp b/logd/PkgIds.cpp
new file mode 100644
index 0000000..2c7c8a6
--- /dev/null
+++ b/logd/PkgIds.cpp
@@ -0,0 +1,178 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <sys/inotify.h>
+#include <sys/prctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <atomic>
+#include <map>
+#include <mutex>
+#include <string>
+#include <thread>
+
+#include <android-base/logging.h>
+#include <android-base/unique_fd.h>
+#include <packagelistparser/packagelistparser.h>
+
+#include "LogUtils.h"
+
+using android::base::unique_fd;
+
+#define PACKAGES_LIST_FILE "/data/system/packages.list"
+
+struct PkgIdMap {
+ bool active = false;
+ std::mutex lock;
+ std::atomic<bool> running = false;
+ std::map<uid_t, std::string> appid_to_package;
+ std::unique_ptr<std::thread> pklp_thread;
+};
+
+static PkgIdMap* gPkgIdMap = new PkgIdMap;
+
+static bool PackageParseCallback(pkg_info* info, void* userdata) {
+ struct PkgIdMap* global = static_cast<struct PkgIdMap*>(userdata);
+ if (info->name != NULL && info->name[0] != '\0') {
+ global->appid_to_package.emplace(info->uid, info->name);
+ }
+ packagelist_free(info);
+ return true;
+}
+
+static bool ReadPackageList(struct PkgIdMap* global) {
+ std::lock_guard<std::mutex> guard(global->lock);
+ global->appid_to_package.clear();
+ bool rc = packagelist_parse(PackageParseCallback, global);
+ LOG(INFO) << "ReadPackageList, total packages: " << global->appid_to_package.size();
+ return rc;
+}
+
+static void WatchPackageList(struct PkgIdMap* global) {
+ struct inotify_event* event;
+ char event_buf[512];
+
+ unique_fd nfd(inotify_init1(IN_CLOEXEC));
+ if (nfd == -1) {
+ LOG(FATAL) << "inotify_init failed";
+ return;
+ }
+
+ global->active = false;
+ while (1) {
+ if (!global->active) {
+ LOG(INFO) << "start watching " PACKAGES_LIST_FILE " ...";
+ int ret = inotify_add_watch(nfd, PACKAGES_LIST_FILE, IN_DELETE_SELF);
+ if (ret == -1) {
+ if (errno == ENOENT || errno == EACCES) {
+ LOG(INFO) << "missing " PACKAGES_LIST_FILE "; retrying...";
+ return;
+ } else {
+ PLOG(ERROR) << "inotify_add_watch failed";
+ return;
+ }
+ }
+
+ if (ReadPackageList(global) == false) {
+ LOG(ERROR) << "ReadPackageList failed";
+ return;
+ }
+ global->active = true;
+ }
+
+ int event_pos = 0;
+ ssize_t res = TEMP_FAILURE_RETRY(read(nfd, event_buf, sizeof(event_buf)));
+ if (res == -1 || static_cast<size_t>(res) < sizeof(*event)) {
+ LOG(ERROR) << "failed to read inotify event, res: " << res;
+ global->active = false;
+ continue;
+ }
+
+ while (res >= static_cast<ssize_t>(sizeof(*event))) {
+ int event_size;
+ event = reinterpret_cast<struct inotify_event*>(event_buf + event_pos);
+
+ if ((event->mask & IN_IGNORED) == IN_IGNORED) {
+ global->active = false;
+ }
+
+ event_size = sizeof(*event) + event->len;
+ res -= event_size;
+ event_pos += event_size;
+ }
+ }
+}
+
+static void StartHandler(struct PkgIdMap* global) {
+ prctl(PR_SET_NAME, "logd.pkglist");
+ WatchPackageList(global);
+ global->running = false;
+}
+
+void StartPkgMonitor() {
+ std::lock_guard<std::mutex> guard(gPkgIdMap->lock);
+ if (gPkgIdMap->pklp_thread.get() == nullptr) {
+ gPkgIdMap->running = true;
+ gPkgIdMap->pklp_thread = std::make_unique<std::thread>(StartHandler, gPkgIdMap);
+ } else if (!gPkgIdMap->running) {
+ gPkgIdMap->pklp_thread->join();
+ gPkgIdMap->running = true;
+ gPkgIdMap->pklp_thread.reset(new std::thread(StartHandler, gPkgIdMap));
+ }
+}
+
+char* android::uidToName(uid_t u) {
+ {
+ std::lock_guard<std::mutex> guard(gPkgIdMap->lock);
+ if (gPkgIdMap->active) {
+ const auto& iter = gPkgIdMap->appid_to_package.find(u);
+ if (iter != gPkgIdMap->appid_to_package.end()) {
+ return strdup(iter->second.c_str());
+ }
+ }
+ }
+
+ struct Userdata {
+ uid_t uid;
+ char* name;
+ } userdata = {
+ .uid = u,
+ .name = nullptr,
+ };
+
+ packagelist_parse(
+ [](pkg_info* info, void* callback_parameter) {
+ auto userdata = reinterpret_cast<Userdata*>(callback_parameter);
+ bool result = true;
+ if (info->uid == userdata->uid) {
+ userdata->name = strdup(info->name);
+ // false to stop processing
+ result = false;
+ }
+ packagelist_free(info);
+ return result;
+ },
+ &userdata);
+
+ if (userdata.name != nullptr) {
+ StartPkgMonitor();
+ }
+ return userdata.name;
+}
diff --git a/logd/SerializedFlushToState.cpp b/logd/SerializedFlushToState.cpp
index 30276ff..a514772 100644
--- a/logd/SerializedFlushToState.cpp
+++ b/logd/SerializedFlushToState.cpp
@@ -34,7 +34,7 @@
SerializedFlushToState::~SerializedFlushToState() {
log_id_for_each(i) {
if (log_positions_[i]) {
- log_positions_[i]->buffer_it->DecReaderRefCount();
+ log_positions_[i]->buffer_it->DetachReader(this);
}
}
}
@@ -49,7 +49,7 @@
if (it == logs_[log_id].end()) {
--it;
}
- it->IncReaderRefCount();
+ it->AttachReader(this);
log_position.buffer_it = it;
// Find the offset of the first log with sequence number >= start().
@@ -80,9 +80,9 @@
logs_needed_from_next_position_[log_id] = true;
} else {
// Otherwise, if there is another buffer piece, move to that and do the same check.
- buffer_it->DecReaderRefCount();
+ buffer_it->DetachReader(this);
++buffer_it;
- buffer_it->IncReaderRefCount();
+ buffer_it->AttachReader(this);
log_positions_[log_id]->read_offset = 0;
if (buffer_it->write_offset() == 0) {
logs_needed_from_next_position_[log_id] = true;
@@ -145,15 +145,11 @@
return {log_id, entry};
}
-void SerializedFlushToState::Prune(log_id_t log_id,
- const std::list<SerializedLogChunk>::iterator& buffer_it) {
- // If we don't have a position for this log or if we're not referencing buffer_it, ignore.
- if (!log_positions_[log_id].has_value() || log_positions_[log_id]->buffer_it != buffer_it) {
- return;
- }
+void SerializedFlushToState::Prune(log_id_t log_id) {
+ CHECK(log_positions_[log_id].has_value());
// Decrease the ref count since we're deleting our reference.
- buffer_it->DecReaderRefCount();
+ log_positions_[log_id]->buffer_it->DetachReader(this);
// Delete in the reference.
log_positions_[log_id].reset();
diff --git a/logd/SerializedFlushToState.h b/logd/SerializedFlushToState.h
index 1a69f7b..8a0d0c5 100644
--- a/logd/SerializedFlushToState.h
+++ b/logd/SerializedFlushToState.h
@@ -61,8 +61,7 @@
// If the parent log buffer prunes logs, the reference that this class contains may become
// invalid, so this must be called first to drop the reference to buffer_it, if any.
- void Prune(log_id_t log_id, const std::list<SerializedLogChunk>::iterator& buffer_it)
- REQUIRES(logd_lock);
+ void Prune(log_id_t log_id) REQUIRES(logd_lock);
private:
// Set logs_needed_from_next_position_[i] to indicate if log_positions_[i] points to an unread
diff --git a/logd/SerializedFlushToStateTest.cpp b/logd/SerializedFlushToStateTest.cpp
index f41de37..12904cb 100644
--- a/logd/SerializedFlushToStateTest.cpp
+++ b/logd/SerializedFlushToStateTest.cpp
@@ -306,5 +306,5 @@
auto state = SerializedFlushToState{1, kLogMaskAll, log_chunks};
ASSERT_TRUE(state.HasUnreadLogs());
- state.Prune(LOG_ID_MAIN, log_chunks[LOG_ID_MAIN].begin());
+ state.Prune(LOG_ID_MAIN);
}
diff --git a/logd/SerializedLogBuffer.cpp b/logd/SerializedLogBuffer.cpp
index aa80864..7446813 100644
--- a/logd/SerializedLogBuffer.cpp
+++ b/logd/SerializedLogBuffer.cpp
@@ -131,14 +131,6 @@
chunk.DecReaderRefCount();
}
-void SerializedLogBuffer::NotifyReadersOfPrune(
- log_id_t log_id, const std::list<SerializedLogChunk>::iterator& chunk) {
- for (const auto& reader_thread : reader_list_->reader_threads()) {
- auto& state = reinterpret_cast<SerializedFlushToState&>(reader_thread->flush_to_state());
- state.Prune(log_id, chunk);
- }
-}
-
void SerializedLogBuffer::Prune(log_id_t log_id, size_t bytes_to_free, uid_t uid) {
auto& log_buffer = logs_[log_id];
auto it = log_buffer.begin();
@@ -174,7 +166,7 @@
// Readers may have a reference to the chunk to track their last read log_position.
// Notify them to delete the reference.
- NotifyReadersOfPrune(log_id, it_to_prune);
+ it_to_prune->NotifyReadersOfPrune(log_id);
if (uid != 0) {
// Reorder the log buffer to remove logs from the given UID. If there are no logs left
diff --git a/logd/SerializedLogBuffer.h b/logd/SerializedLogBuffer.h
index 239a81e..4ce7a3b 100644
--- a/logd/SerializedLogBuffer.h
+++ b/logd/SerializedLogBuffer.h
@@ -58,8 +58,6 @@
bool ShouldLog(log_id_t log_id, const char* msg, uint16_t len);
void MaybePrune(log_id_t log_id) REQUIRES(logd_lock);
void Prune(log_id_t log_id, size_t bytes_to_free, uid_t uid) REQUIRES(logd_lock);
- void NotifyReadersOfPrune(log_id_t log_id, const std::list<SerializedLogChunk>::iterator& chunk)
- REQUIRES(logd_lock);
void RemoveChunkFromStats(log_id_t log_id, SerializedLogChunk& chunk);
size_t GetSizeUsed(log_id_t id) REQUIRES(logd_lock);
diff --git a/logd/SerializedLogChunk.cpp b/logd/SerializedLogChunk.cpp
index 1ffe7a8..a512bf3 100644
--- a/logd/SerializedLogChunk.cpp
+++ b/logd/SerializedLogChunk.cpp
@@ -19,6 +19,7 @@
#include <android-base/logging.h>
#include "CompressionEngine.h"
+#include "SerializedFlushToState.h"
SerializedLogChunk::~SerializedLogChunk() {
CHECK_EQ(reader_ref_count_, 0U);
@@ -52,6 +53,26 @@
}
}
+void SerializedLogChunk::AttachReader(SerializedFlushToState* reader) {
+ readers_.emplace_back(reader);
+ IncReaderRefCount();
+}
+
+void SerializedLogChunk::DetachReader(SerializedFlushToState* reader) {
+ auto it = std::find(readers_.begin(), readers_.end(), reader);
+ CHECK(readers_.end() != it);
+ readers_.erase(it);
+ DecReaderRefCount();
+}
+
+void SerializedLogChunk::NotifyReadersOfPrune(log_id_t log_id) {
+ // Readers will call DetachReader() in their Prune() call, so we make a copy of the list first.
+ auto readers = readers_;
+ for (auto& reader : readers) {
+ reader->Prune(log_id);
+ }
+}
+
bool SerializedLogChunk::ClearUidLogs(uid_t uid, log_id_t log_id, LogStatistics* stats) {
CHECK_EQ(reader_ref_count_, 0U);
if (write_offset_ == 0) {
diff --git a/logd/SerializedLogChunk.h b/logd/SerializedLogChunk.h
index 645433d..2318a2d 100644
--- a/logd/SerializedLogChunk.h
+++ b/logd/SerializedLogChunk.h
@@ -18,12 +18,17 @@
#include <sys/types.h>
+#include <vector>
+
#include <android-base/logging.h>
#include "LogWriter.h"
+#include "LogdLock.h"
#include "SerializedData.h"
#include "SerializedLogEntry.h"
+class SerializedFlushToState;
+
class SerializedLogChunk {
public:
explicit SerializedLogChunk(size_t size) : contents_(size) {}
@@ -33,6 +38,10 @@
void Compress();
void IncReaderRefCount();
void DecReaderRefCount();
+ void AttachReader(SerializedFlushToState* reader);
+ void DetachReader(SerializedFlushToState* reader);
+
+ void NotifyReadersOfPrune(log_id_t log_id) REQUIRES(logd_lock);
// Must have no readers referencing this. Return true if there are no logs left in this chunk.
bool ClearUidLogs(uid_t uid, log_id_t log_id, LogStatistics* stats);
@@ -76,4 +85,5 @@
bool writer_active_ = true;
uint64_t highest_sequence_number_ = 1;
SerializedData compressed_log_;
+ std::vector<SerializedFlushToState*> readers_;
};
diff --git a/logd/AndroidTest.xml b/logd/device_test_config.xml
similarity index 100%
rename from logd/AndroidTest.xml
rename to logd/device_test_config.xml
diff --git a/logd/logd_test.cpp b/logd/logd_test.cpp
index 828f580..ec09cd7 100644
--- a/logd/logd_test.cpp
+++ b/logd/logd_test.cpp
@@ -32,6 +32,7 @@
#include <android-base/stringprintf.h>
#include <android-base/unique_fd.h>
#include <cutils/sockets.h>
+#include <gtest/gtest-death-test.h>
#include <gtest/gtest.h>
#include <log/log_read.h>
#include <private/android_filesystem_config.h>
@@ -843,9 +844,9 @@
TEST(logd, no_epipe) {
#ifdef __ANDROID__
// Actually generating SIGPIPE in logd is racy, since we need to close the socket quicker than
- // logd finishes writing the data to it, so we try 10 times, which should be enough to trigger
+ // logd finishes writing the data to it, so we try 5 times, which should be enough to trigger
// SIGPIPE if logd isn't ignoring SIGPIPE
- for (int i = 0; i < 10; ++i) {
+ for (int i = 0; i < 5; ++i) {
unique_fd sock1(
socket_local_client("logd", ANDROID_SOCKET_NAMESPACE_RESERVED, SOCK_STREAM));
ASSERT_GT(sock1, 0);
@@ -861,7 +862,7 @@
struct pollfd p = {.fd = sock2, .events = POLLIN, .revents = 0};
- int ret = poll(&p, 1, 20);
+ int ret = poll(&p, 1, 1000);
EXPECT_EQ(ret, 1);
EXPECT_TRUE(p.revents & POLLIN);
EXPECT_FALSE(p.revents & POLL_ERR);
@@ -870,3 +871,39 @@
GTEST_LOG_(INFO) << "This test does nothing.\n";
#endif
}
+
+// Only AID_ROOT, AID_SYSTEM, and AID_LOG can set log sizes. Ensure that a different user, AID_BIN,
+// cannot set the log size.
+TEST(logd, logging_permissions) {
+#ifdef __ANDROID__
+ if (getuid() != 0) {
+ GTEST_SKIP() << "This test requires root";
+ }
+
+ auto child_main = [] {
+ setgroups(0, nullptr);
+ setgid(AID_BIN);
+ setuid(AID_BIN);
+
+ std::unique_ptr<logger_list, decltype(&android_logger_list_free)> logger_list{
+ android_logger_list_alloc(0, 0, 0), &android_logger_list_free};
+ if (!logger_list) {
+ _exit(1);
+ }
+ auto logger = android_logger_open(logger_list.get(), LOG_ID_MAIN);
+ if (!logger) {
+ _exit(2);
+ }
+ // This line should fail, so if it returns 0, we exit with an error.
+ if (android_logger_set_log_size(logger, 2 * 1024 * 1024) == 0) {
+ _exit(3);
+ }
+ _exit(EXIT_SUCCESS);
+ };
+
+ ASSERT_EXIT(child_main(), testing::ExitedWithCode(0), "");
+
+#else
+ GTEST_LOG_(INFO) << "This test does nothing.\n";
+#endif
+}
diff --git a/logd/main.cpp b/logd/main.cpp
index c92c5b7..d0d2ac7 100644
--- a/logd/main.cpp
+++ b/logd/main.cpp
@@ -43,7 +43,6 @@
#include <cutils/android_get_control_file.h>
#include <cutils/sockets.h>
#include <log/event_tag_map.h>
-#include <packagelistparser/packagelistparser.h>
#include <private/android_filesystem_config.h>
#include <private/android_logger.h>
#include <processgroup/sched_policy.h>
@@ -79,11 +78,6 @@
PLOG(FATAL) << "failed to set background scheduling policy";
}
- sched_param param = {};
- if (sched_setscheduler((pid_t)0, SCHED_BATCH, ¶m) < 0) {
- PLOG(FATAL) << "failed to set batch scheduler";
- }
-
if (!GetBoolProperty("ro.debuggable", false)) {
if (prctl(PR_SET_DUMPABLE, 0) == -1) {
PLOG(FATAL) << "failed to clear PR_SET_DUMPABLE";
@@ -121,32 +115,6 @@
return GetBoolProperty(name, default_value);
}
-char* android::uidToName(uid_t u) {
- struct Userdata {
- uid_t uid;
- char* name;
- } userdata = {
- .uid = u,
- .name = nullptr,
- };
-
- packagelist_parse(
- [](pkg_info* info, void* callback_parameter) {
- auto userdata = reinterpret_cast<Userdata*>(callback_parameter);
- bool result = true;
- if (info->uid == userdata->uid) {
- userdata->name = strdup(info->name);
- // false to stop processing
- result = false;
- }
- packagelist_free(info);
- return result;
- },
- &userdata);
-
- return userdata.name;
-}
-
static void readDmesg(LogAudit* al, LogKlog* kl) {
if (!al && !kl) {
return;
diff --git a/logwrapper/logwrap.cpp b/logwrapper/logwrap.cpp
index 5a518bc..47d1c22 100644
--- a/logwrapper/logwrap.cpp
+++ b/logwrapper/logwrap.cpp
@@ -161,7 +161,7 @@
if (log_info->log_target & LOG_ALOG) {
ALOG(LOG_INFO, log_info->btag, "%s", line);
}
- if (log_info->log_target & LOG_FILE) {
+ if (log_info->fp) {
fprintf(log_info->fp, "%s\n", line);
}
}
@@ -338,7 +338,7 @@
int rc = 0;
int fd;
- struct log_info log_info;
+ struct log_info log_info = {};
int a = 0; // start index of unprocessed data
int b = 0; // end index of unprocessed data
@@ -510,8 +510,8 @@
err_waitpid:
err_poll:
- if (log_target & LOG_FILE) {
- fclose(log_info.fp); /* Also closes underlying fd */
+ if (log_info.fp) {
+ fclose(log_info.fp);
}
if (abbreviated) {
free_abbr_buf(&log_info.a_buf);
