Suppress a couple of coverity false alarms in FileLoggerOpen().

The stat() system call is used to determine whether to rotate logs.
There is no danger of time-of-check vs. time-of-use because the rotation
decision still holds even under the very-unlikely condition that the existing
log file size changes.

When rotating the logs, the service should not stop when a rename() fails
on an old file.  The process ignores the rename() return code intentionally.
The error condition cannot be logged because the process is already in the
log handling context and would either crash or risk a recursion loop
otherwise.  In addition, writing to stdout/stderr is useless, since the
process is running as a service and the stdout/stderr is reopened on /dev/null.

Therefore, the above two coverity issues are suppressed in the code.

diff --git a/open-vm-tools/lib/glibUtils/fileLogger.c b/open-vm-tools/lib/glibUtils/fileLogger.c
index ace171d1ecf06832a9ad8d4793f3959a8d3f4f03..65f4e7f9820dbfc14ed4fa0912057878b9866e12 100644 (file)
--- a/open-vm-tools/lib/glibUtils/fileLogger.c
+++ b/open-vm-tools/lib/glibUtils/fileLogger.c
@@ -247,6 +247,15 @@ FileLoggerOpen(FileLogger *data)
       struct stat fstats;
 #endif
 
+      /*
+       * In order to determine whether we should rotate the logs,
+       * we are calling the system call stat() to get the existing log file
+       * size.
+       * The time of check vs. time of use issue does not apply to this use
+       * case, as even the file size is increasing, it will not affect the log
+       * rotation decision. So Suppress the fs_check_call coverity warning.
+       */
+      /* coverity[fs_check_call] */
       if (g_stat(path, &fstats) > -1) {
          data->logSize = (gint) fstats.st_size;
       }
@@ -283,6 +292,13 @@ FileLoggerOpen(FileLogger *data)
             if (!g_file_test(dest, G_FILE_TEST_IS_DIR) &&
                 (!g_file_test(dest, G_FILE_TEST_EXISTS) ||
                  g_unlink(dest) == 0)) {
+               /*
+                * We should ignore an unlikely rename() system call failure,
+                * as we should keep our service running with non-critical errors.
+                * We cannot log the error because we are already in the log
+                * handler context to avoid crash or recursive logging loop.
+                */
+               /* coverity[check_return] */
                g_rename(src, dest);
             } else {
                g_unlink(src);
@@ -311,6 +327,7 @@ FileLoggerOpen(FileLogger *data)
 #ifdef _WIN32
       (void) Win32Access_SetFileOwnerRW(path);
 #else
+      /* coverity[toctou] */
       (void) chmod(path, 0600);
 #endif
 #endif // VMX86_TOOLS

diff --git a/open-vm-tools/vgauth/service/fileLogger.c b/open-vm-tools/vgauth/service/fileLogger.c
index eca231d89175ab067ae0c512147637437961e697..e3800770055bff1edb44756238422db3db05d955 100644 (file)
--- a/open-vm-tools/vgauth/service/fileLogger.c
+++ b/open-vm-tools/vgauth/service/fileLogger.c
@@ -76,6 +76,15 @@ ServiceFileLoggerOpen(FileLoggerData *data)
       /* GStatBuf was added in 2.26. */
       GStatBuf fstats;
 
+      /*
+       * In order to determine whether we should rotate the logs,
+       * we are calling the system call stat() to get the existing log file
+       * size.
+       * The time of check vs. time of use issue does not apply to this use
+       * case, as even the file size is increasing, it will not affect the log
+       * rotation decision. So Suppress the fs_check_call coverity warning.
+       */
+      /* coverity[fs_check_call] */
       if (g_stat(path, &fstats) > -1) {
          g_atomic_int_set(&data->logSize, (gint) fstats.st_size);
       }
@@ -112,6 +121,13 @@ ServiceFileLoggerOpen(FileLoggerData *data)
             if (!g_file_test(dest, G_FILE_TEST_IS_DIR) &&
                 (!g_file_test(dest, G_FILE_TEST_EXISTS) ||
                  g_unlink(dest) == 0)) {
+               /*
+                * We should ignore an unlikely rename() system call failure,
+                * as we should keep our service running with non-critical errors.
+                * We cannot log the error because we are already in the log
+                * handler context to avoid crash or recursive logging loop.
+                */
+               /* coverity[check_return] */
                g_rename(src, dest);
             } else {
                g_unlink(src);
@@ -128,6 +144,7 @@ ServiceFileLoggerOpen(FileLoggerData *data)
       }
    }
 
+   /* coverity[toctou] */
    logfile = g_fopen(path, data->append ? "a" : "w");
    /*
     * Make log readable only by root/Administrator.  Just log any error;