From dfbf1cfba649eb8870b4732db0810d9cc9a6372d Mon Sep 17 00:00:00 2001 From: Marcelo Lynch Date: Thu, 21 Sep 2023 13:01:00 -0700 Subject: [PATCH] Restructure sandbox report for the sake of truncation --- .../Src/Engine/Processes/SandboxConnectionLinuxDetours.cs | 4 ++-- Public/Src/Sandbox/Linux/bxl_observer.cpp | 7 +++++-- Public/Src/Sandbox/Linux/bxl_observer.hpp | 6 ++++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/Public/Src/Engine/Processes/SandboxConnectionLinuxDetours.cs b/Public/Src/Engine/Processes/SandboxConnectionLinuxDetours.cs index 059ed7fb78..e23f6f5ec0 100644 --- a/Public/Src/Engine/Processes/SandboxConnectionLinuxDetours.cs +++ b/Public/Src/Engine/Processes/SandboxConnectionLinuxDetours.cs @@ -384,7 +384,7 @@ private void ProcessBytes((PooledObjectWrapper wrapper, int length) item var message = s_encoding.GetString(item.wrapper.Instance, index: 0, count: item.length).AsSpan().TrimEnd('\n'); // parse the message, consuming the span field by field. The format is: - // "%s|%d|%d|%d|%d|%d|%d|%s|%d\n", __progname, getpid(), access, status, explicitLogging, err, opcode, reportPath, isDirectory + // "%s|%d|%d|%d|%d|%d|%d|%d|%s\n", __progname, getpid(), access, status, explicitLogging, err, opcode, isDirectory, reportPath var restOfMessage = message; _ = nextField(restOfMessage, out restOfMessage); // ignore progname var pid = AssertInt(nextField(restOfMessage, out restOfMessage)); @@ -393,8 +393,8 @@ private void ProcessBytes((PooledObjectWrapper wrapper, int length) item var explicitlogging = AssertInt(nextField(restOfMessage, out restOfMessage)); var err = AssertInt(nextField(restOfMessage, out restOfMessage)); var opCode = AssertInt(nextField(restOfMessage, out restOfMessage)); - var path = nextField(restOfMessage, out restOfMessage); var isDirectory = AssertInt(nextField(restOfMessage, out restOfMessage)); + var path = nextField(restOfMessage, out restOfMessage); Contract.Assert(restOfMessage.IsEmpty); // We should have reached the end of the message // ignore accesses to libDetours.so, because we injected that library diff --git a/Public/Src/Sandbox/Linux/bxl_observer.cpp b/Public/Src/Sandbox/Linux/bxl_observer.cpp index 5383123f59..e1a2b19f51 100644 --- a/Public/Src/Sandbox/Linux/bxl_observer.cpp +++ b/Public/Src/Sandbox/Linux/bxl_observer.cpp @@ -362,8 +362,11 @@ bool BxlObserver::SendReport(const AccessReport &report, bool isDebugMessage, bo } else { - // The debug message was truncated. Let's crop the debug message so it fits - int truncatedSize = PATH_MAX - (numWritten - maxMessageLength); + // The report couldn't be fully built for a debug message. Let's crop the message so it fits. + // We calculate the maximum size allowed, considering that 'path' is the last component of the + // message (plus the \n that ends any report, hence the -1), so it's the last thing + // we tried to write when hitting the size limit. + int truncatedSize = PATH_MAX - (numWritten - maxMessageLength) - 1; char truncatedMessage[truncatedSize] = {0}; // Let's leave an ending \0 diff --git a/Public/Src/Sandbox/Linux/bxl_observer.hpp b/Public/Src/Sandbox/Linux/bxl_observer.hpp index 72f8fd1886..bd471f194c 100644 --- a/Public/Src/Sandbox/Linux/bxl_observer.hpp +++ b/Public/Src/Sandbox/Linux/bxl_observer.hpp @@ -328,9 +328,11 @@ class BxlObserver final // Builds the report to be sent over the FIFO in the given buffer inline int BuildReport(char* buffer, int maxMessageLength, const AccessReport &report, const char *path) { + // Note: when adding new fields, always leave 'path' as the last component of this message + // This is for the sake of the arithmetic when truncating debug messages, where this assumption is made (see SendReport). return snprintf( - buffer, maxMessageLength, "%s|%d|%d|%d|%d|%d|%d|%s|%d\n", - __progname, report.pid <= 0 ? getpid() : report.pid, report.requestedAccess, report.status, report.reportExplicitly, report.error, report.operation, path, report.isDirectory); + buffer, maxMessageLength, "%s|%d|%d|%d|%d|%d|%d|%d|%s\n", + __progname, report.pid <= 0 ? getpid() : report.pid, report.requestedAccess, report.status, report.reportExplicitly, report.error, report.operation, report.isDirectory, path); } static BxlObserver *sInstance;