Skip to content

Commit 7863acd

Browse files
authored
Static Analysis Fixes (#478)
Signed-off-by: Russell McGuire <russell.w.mcguire@intel.com>
1 parent a489a42 commit 7863acd

2 files changed

Lines changed: 36 additions & 21 deletions

File tree

source/utils/ze_logger.cpp

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,6 @@ struct LogSink {
156156
is_good(true)
157157
{}
158158

159-
bool good() const {
160-
return is_good;
161-
}
162-
163159
void write(const std::string &line) {
164160
std::lock_guard<std::mutex> lk(mtx);
165161
if (is_good) {
@@ -550,44 +546,52 @@ std::shared_ptr<ZeLogger> createLogger(const std::string &caller) {
550546
output_dest = "stderr (console)";
551547
} else {
552548
// Create the full directory path (equivalent to mkdir -p).
549+
// Each component is created with mkdir-first, then verified on EEXIST.
550+
// This avoids a check-then-create TOCTOU window: an attacker cannot win
551+
// a race between an existence test and the create call because there is
552+
// no separate existence test — mkdir itself is the atomic decision.
553+
// We still use stat()/GetFileAttributesA (which follow symlinks) so that
554+
// existing symlinks-to-directories continue to work as before.
553555
#ifdef _WIN32
554-
// Walk each component and create it if missing.
555556
for (std::size_t pos = 0; pos <= log_directory.size(); ++pos) {
556557
if (pos == log_directory.size() ||
557558
log_directory[pos] == '\\' || log_directory[pos] == '/') {
558559
if (pos == 0) continue;
559560
std::string partial = log_directory.substr(0, pos);
560-
DWORD attrs = GetFileAttributesA(partial.c_str());
561-
if (attrs == INVALID_FILE_ATTRIBUTES) {
562-
if (_mkdir(partial.c_str()) != 0 && errno != EEXIST) {
561+
if (_mkdir(partial.c_str()) != 0) {
562+
if (errno != EEXIST) {
563563
std::cerr << "ze_logger: Failed to create log directory '"
564564
<< partial << "': " << errnoToString(errno) << "\n";
565565
return std::shared_ptr<ZeLogger>(new ZeLogger());
566566
}
567-
} else if (!(attrs & FILE_ATTRIBUTE_DIRECTORY)) {
568-
std::cerr << "ze_logger: Log directory path component '"
569-
<< partial << "' exists but is not a directory\n";
570-
return std::shared_ptr<ZeLogger>(new ZeLogger());
567+
// Already exists — verify it is a directory.
568+
DWORD attrs = GetFileAttributesA(partial.c_str());
569+
if (attrs == INVALID_FILE_ATTRIBUTES || !(attrs & FILE_ATTRIBUTE_DIRECTORY)) {
570+
std::cerr << "ze_logger: Log directory path component '"
571+
<< partial << "' exists but is not a directory\n";
572+
return std::shared_ptr<ZeLogger>(new ZeLogger());
573+
}
571574
}
572575
}
573576
}
574577
#else
575-
// Walk each component and create it if missing.
576578
for (std::size_t pos = 0; pos <= log_directory.size(); ++pos) {
577579
if (pos == log_directory.size() || log_directory[pos] == '/') {
578580
if (pos == 0) continue;
579581
std::string partial = log_directory.substr(0, pos);
580-
struct stat st{};
581-
if (stat(partial.c_str(), &st) != 0) {
582-
if (mkdir(partial.c_str(), 0755) != 0 && errno != EEXIST) {
582+
if (mkdir(partial.c_str(), 0755) != 0) {
583+
if (errno != EEXIST) {
583584
std::cerr << "ze_logger: Failed to create log directory '"
584585
<< partial << "': " << errnoToString(errno) << "\n";
585586
return std::shared_ptr<ZeLogger>(new ZeLogger());
586587
}
587-
} else if (!S_ISDIR(st.st_mode)) {
588-
std::cerr << "ze_logger: Log directory path component '"
589-
<< partial << "' exists but is not a directory\n";
590-
return std::shared_ptr<ZeLogger>(new ZeLogger());
588+
// Already exists — verify it is a directory (follows symlinks).
589+
struct stat st{};
590+
if (stat(partial.c_str(), &st) != 0 || !S_ISDIR(st.st_mode)) {
591+
std::cerr << "ze_logger: Log directory path component '"
592+
<< partial << "' exists but is not a directory\n";
593+
return std::shared_ptr<ZeLogger>(new ZeLogger());
594+
}
591595
}
592596
}
593597
}
@@ -600,7 +604,7 @@ std::shared_ptr<ZeLogger> createLogger(const std::string &caller) {
600604
std::string cfg;
601605
cfg = caller + " logging enabled:";
602606
cfg += "\n ZEL_LOADER_LOG_CONSOLE : " + std::string(log_console ? "stderr" : "disabled");
603-
cfg += "\n ZEL_ENABLE_LOADER_LOGGING : " + std::string(logging_enabled ? "enabled" : "disabled");
607+
cfg += "\n ZEL_ENABLE_LOADER_LOGGING : " + std::string(logging_mode == 2 ? "enabled (advanced)" : "enabled");
604608
cfg += "\n ZEL_LOADER_LOGGING_LEVEL : " + log_level;
605609
cfg += "\n ZEL_LOADER_LOG_DIR : " + log_directory;
606610
cfg += "\n ZEL_LOADER_LOG_FILE : " + loader_file;

source/utils/ze_logger.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,17 @@ class ZeLogger {
5555
ZeLogger(bool use_stderr, LogLevel level, const std::string &pattern);
5656
~ZeLogger();
5757

58+
// Non-copyable and non-movable: a logger owns a sink (stream + mutex) and
59+
// is always handled through shared_ptr<ZeLogger> (via createLogger()) or as
60+
// a raw pointer to the process-lifetime noopLogger() instance. Copying
61+
// would imply duplicating the sink (nonsensical for console, racy for
62+
// files); moving is not needed because callers never hold a ZeLogger by
63+
// value. Deleting all four documents intent and satisfies Rule-of-Five.
64+
ZeLogger(const ZeLogger&) = delete;
65+
ZeLogger& operator=(const ZeLogger&) = delete;
66+
ZeLogger(ZeLogger&&) = delete;
67+
ZeLogger& operator=(ZeLogger&&) = delete;
68+
5869
void setLevel(LogLevel level);
5970
LogLevel getLevel() const;
6071

0 commit comments

Comments
 (0)