From fd0b2ecdaa49c8009c7db05a19fc8dbe67f9303e Mon Sep 17 00:00:00 2001
From: Daniel Verkamp <dverkamp@chromium.org>
Date: Fri, 19 Aug 2022 17:08:37 -0700
Subject: [PATCH] base: linux: store syslog fd in a OnceCell

Ensure that openlog_and_get_socket() is only called once by storing its
result in a OnceCell. This fixes logging initialization when the syslog
State structure was constructed more than once, causing the log fd to be
invalidated and breaking sandboxed startup.

BUG=b:242103548
TEST=Start crosvm on Linux with sandbox enabled (default mode)

Change-Id: I663a7e918b81f4c23415a28cfcc2c72f6b4a51d8
Fixes: 7dbdc730636c ("base & src: allow multiple log inits.")
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3842813
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
---
 base/src/sys/unix/linux/syslog.rs | 54 +++++++++++++++++--------------
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/base/src/sys/unix/linux/syslog.rs b/base/src/sys/unix/linux/syslog.rs
index 0855e3df0d..479c6de0d9 100644
--- a/base/src/sys/unix/linux/syslog.rs
+++ b/base/src/sys/unix/linux/syslog.rs
@@ -25,6 +25,7 @@ use libc::LOG_NDELAY;
 use libc::LOG_PERROR;
 use libc::LOG_PID;
 use libc::LOG_USER;
+use once_cell::sync::OnceCell;
 
 use super::super::getpid;
 use crate::syslog::Error;
@@ -35,31 +36,35 @@ use crate::RawDescriptor;
 
 const SYSLOG_PATH: &str = "/dev/log";
 
+/// Global syslog socket derived from the fd opened by `openlog()`.
+/// This is initialized in `PlatformSyslog::new()`.
+static SYSLOG_SOCKET: OnceCell<UnixDatagram> = OnceCell::new();
+
 pub struct PlatformSyslog {}
 
-struct SyslogSocket {
-    socket: UnixDatagram,
-}
+struct SyslogSocket {}
 
 impl Write for SyslogSocket {
     fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
         const SEND_RETRY: usize = 2;
 
-        for _ in 0..SEND_RETRY {
-            match self.socket.send(buf) {
-                Ok(l) => return Ok(l),
-                Err(e) => match e.kind() {
-                    ErrorKind::ConnectionRefused
-                    | ErrorKind::ConnectionReset
-                    | ErrorKind::ConnectionAborted
-                    | ErrorKind::NotConnected => {
-                        let res = self.socket.connect(SYSLOG_PATH);
-                        if res.is_err() {
-                            break;
+        if let Some(socket) = SYSLOG_SOCKET.get() {
+            for _ in 0..SEND_RETRY {
+                match socket.send(buf) {
+                    Ok(l) => return Ok(l),
+                    Err(e) => match e.kind() {
+                        ErrorKind::ConnectionRefused
+                        | ErrorKind::ConnectionReset
+                        | ErrorKind::ConnectionAborted
+                        | ErrorKind::NotConnected => {
+                            let res = socket.connect(SYSLOG_PATH);
+                            if res.is_err() {
+                                break;
+                            }
                         }
-                    }
-                    _ => {}
-                },
+                        _ => {}
+                    },
+                }
             }
         }
         // Abandon all hope but this is fine
@@ -72,24 +77,23 @@ impl Write for SyslogSocket {
 }
 
 impl Syslog for PlatformSyslog {
-    /// WARNING: calling this function more than once will cause the previous
-    /// syslogger FD to be closed, invalidating the log::Log object in an unsafe
-    /// manner. Currently, the callers of this method are extremely careful to
-    /// call it exactly once.
-    ///
-    /// b/238923791 is tracking fixing this problem.
     fn new(
         proc_name: String,
         facility: Facility,
     ) -> Result<(Option<Box<dyn log::Log + Send>>, Option<RawDescriptor>), Error> {
-        let socket = openlog_and_get_socket()?;
+        // Calling openlog_and_get_socket() more than once will cause the previous syslogger FD to
+        // be closed, invalidating the log::Log object in an unsafe manner. The OnceCell
+        // get_or_try_init() ensures we only call it once.
+        //
+        // b/238923791 is tracking fixing this problem.
+        let socket = SYSLOG_SOCKET.get_or_try_init(openlog_and_get_socket)?;
         let mut builder = env_logger::Builder::new();
 
         // Everything is filtered layer above
         builder.filter_level(log::LevelFilter::Trace);
 
         let fd = socket.as_raw_fd();
-        builder.target(env_logger::Target::Pipe(Box::new(SyslogSocket { socket })));
+        builder.target(env_logger::Target::Pipe(Box::new(SyslogSocket {})));
         builder.format(move |buf, record| {
             const MONTHS: [&str; 12] = [
                 "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec",