From c0ca2cbdc2dfcca50d7bc71a93629ca8e02b1682 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Fri, 27 Aug 2021 18:15:13 +0300 Subject: libcamera: base: log: Don't crash when logging after Logger destruction libcamera isn't supposed to log messages after the logger is destroyed, as the global logger instance is destroyed after the main() function returns, and the camera manager is supposed to have been stopped and destroyed before that. This rule is difficult to enforce in the V4L2 compat implementation, as there is no location where we can destroy the camera manager manually before the logger is destroyed. This results in a use-after-free condition when the camera manager gets stopped during destruction. Fix it by not trying to print log messages when the global logger instance has been destroyed. This is a bit of a hack, but hopefully not too bad. There could be race conditions when using a CameraManager instance that is destroyed as part of the destruction of global variables (like the V4L2 compat layer does, it wraps CameraManager in a singleton V4L2CompatManager class, and destroys it when V4L2CompatManager is destroyed) as the CameraManager thread will still be running when the logger gets destroyed, but this doesn't cause any regression as we destroy the logger without any safeguard measure today anyway. There are other options that could be considered. Forcing destruction of the logger after the camera manager in the V4L2 compat layer is one of them, but turned out to be difficult. For instance care would need to be taken *not* to log any message in the mmap() wrapper if the fd doesn't match a wrapped camera, as mmap() is called very early in the initialization process, before libcamera and the logger get initialized. The resulting implementation would likely be fairly complex. Another option could be to wrap the logger with a shared pointer, and keep a reference to it in CameraManager. That's more intrusive, and it's not clear if it would be worth it. Signed-off-by: Laurent Pinchart Reviewed-by: Paul Elder --- src/libcamera/base/log.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 073b7c34..a3e3f9ea 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -270,12 +270,16 @@ private: friend LogCategory; void registerCategory(LogCategory *category); + static bool destroyed_; + std::unordered_set categories_; std::list> levels_; std::shared_ptr output_; }; +bool Logger::destroyed_ = false; + /** * \enum LoggingTarget * \brief Log destination type @@ -372,6 +376,8 @@ void logSetLevel(const char *category, const char *level) Logger::~Logger() { + destroyed_ = true; + for (LogCategory *category : categories_) delete category; } @@ -387,6 +393,10 @@ Logger::~Logger() Logger *Logger::instance() { static Logger instance; + + if (destroyed_) + return nullptr; + return &instance; } @@ -808,13 +818,17 @@ LogMessage::~LogMessage() if (severity_ == LogInvalid) return; + Logger *logger = Logger::instance(); + if (!logger) + return; + msgStream_ << std::endl; if (severity_ >= category_.severity()) - Logger::instance()->write(*this); + logger->write(*this); if (severity_ == LogSeverity::LogFatal) { - Logger::instance()->backtrace(); + logger->backtrace(); std::abort(); } } -- cgit v1.2.1