Commit 8ff8cb8 Ticket 49937 - Log buffer exceeded emergency logging msg is not thread-safe

1 file Authored and Committed by mreynolds 13 days ago
Ticket 49937 - Log buffer exceeded emergency logging msg is not thread-safe

Bug Description:  Multiple operations making modificatiosn on a DN
                  that is very large can crash the server, because
                  when we do emergency logging, we close and reopen
                  the errors log withgout hold the error log write
                  lock.  This causes the FD pointer to be become
                  invalid and triggers a crash.

Fix description:  Hold the errors log write lock while closing and
                  reopening the log

https://pagure.io/389-ds-base/issue/49937

Reviewed by: vashirov(Thanks!)

    
 1 @@ -2231,11 +2231,11 @@
 2       if (logging_hr_timestamps_enabled == 1) {
 3           struct timespec tsnow;
 4           if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) {
 5 -             syslog(LOG_EMERG, "vslapd_log_emergency_error, Unable to determine system time for message :: %s", msg);
 6 +             syslog(LOG_EMERG, "vslapd_log_emergency_error, Unable to determine system time for message :: %s\n", msg);
 7               return;
 8           }
 9           if (format_localTime_hr_log(tsnow.tv_sec, tsnow.tv_nsec, sizeof(tbuf), tbuf, &size) != 0) {
10 -             syslog(LOG_EMERG, "vslapd_log_emergency_error, Unable to format system time for message :: %s", msg);
11 +             syslog(LOG_EMERG, "vslapd_log_emergency_error, Unable to format system time for message :: %s\n", msg);
12               return;
13           }
14       } else {
15 @@ -2243,14 +2243,14 @@
16           time_t tnl;
17           tnl = slapi_current_utc_time();
18           if (format_localTime_log(tnl, sizeof(tbuf), tbuf, &size) != 0) {
19 -             syslog(LOG_EMERG, "vslapd_log_emergency_error, Unable to format system time for message :: %s", msg);
20 +             syslog(LOG_EMERG, "vslapd_log_emergency_error, Unable to format system time for message :: %s\n", msg);
21               return;
22           }
23   #ifdef HAVE_CLOCK_GETTIME
24       }
25   #endif
26   
27 -     PR_snprintf(buffer, sizeof(buffer), "%s - EMERG - %s", tbuf, msg);
28 +     PR_snprintf(buffer, sizeof(buffer), "%s - EMERG - %s\n", tbuf, msg);
29       size = strlen(buffer);
30   
31       if (!locked) {
32 @@ -2531,7 +2531,7 @@
33   
34       if (SLAPI_LOG_BUFSIZ - blen < vlen) {
35           /* We won't be able to fit the message in! Uh-oh! */
36 -         /* Should we actually just do the snprintf, and warn that message was trunced? */
37 +         /* Should we actually just do the snprintf, and warn that message was truncated? */
38           log__error_emergency("Insufficent buffer capacity to fit timestamp and message!", 1, 0);
39           return -1;
40       }
41 @@ -4486,6 +4486,13 @@
42       if (!reopen) {
43           return;
44       }
45 +     if (!locked) {
46 +         /*
47 +          * Take the lock because we are closing and reopening the error log (fd),
48 +          * and we don't want any other threads trying to use this fd
49 +          */
50 +         LOG_ERROR_LOCK_WRITE();
51 +     }
52       if (NULL != loginfo.log_error_fdes) {
53           LOG_CLOSE(loginfo.log_error_fdes);
54       }
55 @@ -4494,7 +4501,10 @@
56           PRErrorCode prerr = PR_GetError();
57           syslog(LOG_ERR, "Failed to reopen errors log file, " SLAPI_COMPONENT_NAME_NSPR " error %d (%s)\n", prerr, slapd_pr_strerror(prerr));
58       } else {
59 -         vslapd_log_emergency_error(loginfo.log_error_fdes, errstr, locked);
60 +         vslapd_log_emergency_error(loginfo.log_error_fdes, errstr, 1 /* locked */);
61 +     }
62 +     if (!locked) {
63 +         LOG_ERROR_UNLOCK_WRITE();
64       }
65       return;
66   }