From ea1d8f637677c216aa2ab41eb351495d332ab304 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= <maze@google.com>
Date: Thu, 5 Sep 2024 09:38:14 -0700
Subject: [PATCH] NetBpfLoad: fix empty log_buf detection logic
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

log_buf.size() was always BPF_LOAD_LOG_SZ and thus != 0

After changing ALOGD to ALOGI you now see (for example):
  NetBpfLoad: BPF_PROG_LOAD call for /apex/com.android.tethering/etc/bpf/net_shared/block.o (bind4_block_port) returned 'processed 55 insns (limit 1000000) max_states_per_insn 0 total_states 4 peak_states 4 mark_read 1' fd: 6 (ok)

(ALOGV/ALOGD's are effectively silently dropped by the KernelLogger we use)

Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I4a296976ae3f1e2217c46d81ed48850fdfa95a3c
---
 bpf/loader/NetBpfLoad.cpp | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp
index 31f40d61d2..69f1cb5b58 100644
--- a/bpf/loader/NetBpfLoad.cpp
+++ b/bpf/loader/NetBpfLoad.cpp
@@ -141,9 +141,6 @@ inline bool isUserdebug() {
 
 #define BPF_FS_PATH "/sys/fs/bpf/"
 
-// Size of the BPF log buffer for verifier logging
-#define BPF_LOAD_LOG_SZ 0xfffff
-
 static unsigned int page_size = static_cast<unsigned int>(getpagesize());
 
 constexpr const char* lookupSelinuxContext(const domain d) {
@@ -1006,7 +1003,7 @@ static int loadCodeSections(const char* elfPath, vector<codeSection>& cs, const
                   (!fd.ok() ? std::strerror(errno) : "no error"));
             reuse = true;
         } else {
-            vector<char> log_buf(BPF_LOAD_LOG_SZ, 0);
+            static char log_buf[1 << 20];  // 1 MiB logging buffer
 
             union bpf_attr req = {
               .prog_type = cs[i].type,
@@ -1014,8 +1011,8 @@ static int loadCodeSections(const char* elfPath, vector<codeSection>& cs, const
               .insns = ptr_to_u64(cs[i].data.data()),
               .license = ptr_to_u64(license.c_str()),
               .log_level = 1,
-              .log_size = static_cast<__u32>(log_buf.size()),
-              .log_buf = ptr_to_u64(log_buf.data()),
+              .log_size = sizeof(log_buf),
+              .log_buf = ptr_to_u64(log_buf),
               .kern_version = kvers,
               .expected_attach_type = cs[i].attach_type,
             };
@@ -1023,12 +1020,23 @@ static int loadCodeSections(const char* elfPath, vector<codeSection>& cs, const
                 strlcpy(req.prog_name, cs[i].name.c_str(), sizeof(req.prog_name));
             fd.reset(bpf(BPF_PROG_LOAD, req));
 
-            ALOGD("BPF_PROG_LOAD call for %s (%s) returned fd: %d (%s)", elfPath,
-                  cs[i].name.c_str(), fd.get(), (!fd.ok() ? std::strerror(errno) : "no error"));
+            // Kernel should have NULL terminated the log buffer, but force it anyway for safety
+            log_buf[sizeof(log_buf) - 1] = 0;
+
+            // Strip out final newline if present
+            int log_chars = strlen(log_buf);
+            if (log_chars && log_buf[log_chars - 1] == '\n') log_buf[--log_chars] = 0;
+
+            bool log_oneline = !strchr(log_buf, '\n');
+
+            ALOGD("BPF_PROG_LOAD call for %s (%s) returned '%s' fd: %d (%s)", elfPath,
+                  cs[i].name.c_str(), log_oneline ? log_buf : "{multiline}",
+                  fd.get(), (!fd.ok() ? std::strerror(errno) : "ok"));
 
             if (!fd.ok()) {
-                if (log_buf.size()) {
-                    vector<string> lines = Split(log_buf.data(), "\n");
+                // kernel NULL terminates log_buf, so this checks for non-empty string
+                if (log_buf[0]) {
+                    vector<string> lines = Split(log_buf, "\n");
 
                     ALOGW("BPF_PROG_LOAD - BEGIN log_buf contents:");
                     for (const auto& line : lines) ALOGW("%s", line.c_str());