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 4940928b8a204b5a949f3da6376b314a032bf85d..2140d22f5f554ddae2705f826698c38eed06907f 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 06ae1c3f504a8e0d459d408438b125e93fb3a7b6..627d54d835a1b1758861f65c71d0cc770ad77060 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;