mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 14:07:59 +00:00
Code Issues Releases Wiki Activity

Fixed data race in log.c

Browse Source 
A data race was happening while BIND was starting due to
isc_log_wouldlog function accessing lctx->logconfig without a lock.

To prevent that without incurring much costs, that variable was made
atomic.
This commit is contained in:
Diego Fronza
2019-12-20 19:29:18 -03:00
committed by Matthijs Mekking
parent 6d252220b0
commit a200852001
1 changed files with 38 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
lib/isc/log.c
View File

@@ -19,6 +19,7 @@
#include <sys/types.h> /* dev_t FreeBSD 2.1 */ #include <sys/types.h> /* dev_t FreeBSD 2.1 */
#include <time.h> #include <time.h>
#include <isc/atomic.h>
#include <isc/dir.h> #include <isc/dir.h>
#include <isc/file.h> #include <isc/file.h>
#include <isc/log.h> #include <isc/log.h>
@@ -138,7 +139,7 @@ struct isc_log {
int debug_level; int debug_level;
isc_mutex_t lock; isc_mutex_t lock;
/* Locked by isc_log lock. */ /* Locked by isc_log lock. */
isc_logconfig_t *logconfig; atomic_uintptr_t logconfig;
char buffer[LOG_BUFFER_SIZE]; char buffer[LOG_BUFFER_SIZE];
ISC_LIST(isc_logmessage_t) messages; ISC_LIST(isc_logmessage_t) messages;
}; };
@@ -276,7 +277,7 @@ isc_log_create(isc_mem_t *mctx, isc_log_t **lctxp, isc_logconfig_t **lcfgp) {
} }
if (result == ISC_R_SUCCESS) { if (result == ISC_R_SUCCESS) {
lctx->logconfig = lcfg; atomic_init(&lctx->logconfig, (uintptr_t)lcfg);
*lctxp = lctx; *lctxp = lctx;
if (lcfgp != NULL) { if (lcfgp != NULL) {
@@ -389,12 +390,8 @@ isc_logconfig_use(isc_log_t *lctx, isc_logconfig_t *lcfg) {
return (result); return (result);
} }
LOCK(&lctx->lock); old_cfg = (isc_logconfig_t *)atomic_exchange_acq_rel(&lctx->logconfig,
(uintptr_t)lcfg);
old_cfg = lctx->logconfig;
lctx->logconfig = lcfg;
UNLOCK(&lctx->lock);
isc_logconfig_destroy(&old_cfg); isc_logconfig_destroy(&old_cfg);
@@ -414,9 +411,9 @@ isc_log_destroy(isc_log_t **lctxp) {
*lctxp = NULL; *lctxp = NULL;
mctx = lctx->mctx; mctx = lctx->mctx;
if (lctx->logconfig != NULL) { lcfg = (isc_logconfig_t *)atomic_exchange_acq_rel(&lctx->logconfig,
lcfg = lctx->logconfig; (uintptr_t)NULL);
lctx->logconfig = NULL; if (lcfg != NULL) {
isc_logconfig_destroy(&lcfg); isc_logconfig_destroy(&lcfg);
} }
@@ -459,7 +456,8 @@ isc_logconfig_destroy(isc_logconfig_t **lcfgp) {
* This function cannot be called with a logconfig that is in * This function cannot be called with a logconfig that is in
* use by a log context. * use by a log context.
*/ */
REQUIRE(lcfg->lctx != NULL && lcfg->lctx->logconfig != lcfg); REQUIRE(lcfg->lctx != NULL &&
atomic_load_acquire(&lcfg->lctx->logconfig) != (uintptr_t)lcfg);
mctx = lcfg->lctx->mctx; mctx = lcfg->lctx->mctx;
@@ -840,6 +838,7 @@ isc_log_setdebuglevel(isc_log_t *lctx, unsigned int level) {
isc_logchannel_t *channel; isc_logchannel_t *channel;
REQUIRE(VALID_CONTEXT(lctx)); REQUIRE(VALID_CONTEXT(lctx));
REQUIRE(atomic_load_acquire(&lctx->logconfig) != (uintptr_t)NULL);
LOCK(&lctx->lock); LOCK(&lctx->lock);
@@ -848,13 +847,16 @@ isc_log_setdebuglevel(isc_log_t *lctx, unsigned int level) {
* Close ISC_LOG_DEBUGONLY channels if level is zero. * Close ISC_LOG_DEBUGONLY channels if level is zero.
*/ */
if (lctx->debug_level == 0) { if (lctx->debug_level == 0) {
for (channel = ISC_LIST_HEAD(lctx->logconfig->channels); isc_logconfig_t *lcfg = (isc_logconfig_t *)atomic_load_acquire(
channel != NULL; channel = ISC_LIST_NEXT(channel, link)) &lctx->logconfig);
{ if (lcfg != NULL) {
if (channel->type == ISC_LOG_TOFILE && for (channel = ISC_LIST_HEAD(lcfg->channels);
(channel->flags & ISC_LOG_DEBUGONLY) != 0 && channel != NULL;
FILE_STREAM(channel) != NULL) channel = ISC_LIST_NEXT(channel, link))
{ {
if (channel->type == ISC_LOG_TOFILE &&
(channel->flags & ISC_LOG_DEBUGONLY) != 0 &&
FILE_STREAM(channel) != NULL)
{ {
(void)fclose(FILE_STREAM(channel)); (void)fclose(FILE_STREAM(channel));
FILE_STREAM(channel) = NULL; FILE_STREAM(channel) = NULL;
@@ -925,12 +927,14 @@ isc_log_closefilelogs(isc_log_t *lctx) {
REQUIRE(VALID_CONTEXT(lctx)); REQUIRE(VALID_CONTEXT(lctx));
LOCK(&lctx->lock); LOCK(&lctx->lock);
for (channel = ISC_LIST_HEAD(lctx->logconfig->channels); isc_logconfig_t *lcfg =
channel != NULL; channel = ISC_LIST_NEXT(channel, link)) (isc_logconfig_t *)atomic_load_acquire(&lctx->logconfig);
{ if (lcfg != NULL) {
if (channel->type == ISC_LOG_TOFILE && for (channel = ISC_LIST_HEAD(lcfg->channels); channel != NULL;
FILE_STREAM(channel) != NULL) { channel = ISC_LIST_NEXT(channel, link))
{ {
if (channel->type == ISC_LOG_TOFILE &&
FILE_STREAM(channel) != NULL) {
(void)fclose(FILE_STREAM(channel)); (void)fclose(FILE_STREAM(channel));
FILE_STREAM(channel) = NULL; FILE_STREAM(channel) = NULL;
} }
@@ -1471,13 +1475,18 @@ isc_log_wouldlog(isc_log_t *lctx, int level) {
* because that's a risk anyway if the logconfig is being * because that's a risk anyway if the logconfig is being
* dynamically changed. * dynamically changed.
*/ */
if (lctx == NULL) {
if (lctx == NULL || lctx->logconfig == NULL) {
return (false); return (false);
} }
return (level <= lctx->logconfig->highest_level || isc_logconfig_t *lcfg =
(lctx->logconfig->dynamic && level <= lctx->debug_level)); (isc_logconfig_t *)atomic_load_acquire(&lctx->logconfig);
if (lcfg == NULL) {
return (false);
}
return (level <= lcfg->highest_level ||
(lcfg->dynamic && level <= lctx->debug_level));
} }
static void static void
@@ -1529,7 +1538,7 @@ isc_log_doit(isc_log_t *lctx, isc_logcategory_t *category,
lctx->buffer[0] = '\0'; lctx->buffer[0] = '\0';
lcfg = lctx->logconfig; lcfg = (isc_logconfig_t *)atomic_load_acquire(&lctx->logconfig);
category_channels = ISC_LIST_HEAD(lcfg->channellists[category->id]); category_channels = ISC_LIST_HEAD(lcfg->channellists[category->id]);