diff --git a/rsyslog-deferred-dns-query-race.diff b/rsyslog-deferred-dns-query-race.diff new file mode 100644 index 0000000..8456680 --- /dev/null +++ b/rsyslog-deferred-dns-query-race.diff @@ -0,0 +1,281 @@ +From 21f69b2c3a95c990ea123d078b08c554cab1d121 Mon Sep 17 00:00:00 2001 +From: Rainer Gerhards +Date: Fri, 8 Apr 2011 11:04:38 +0200 +Subject: [PATCH] bugfix: race condition in deferred name resolution + +Note that this actually is a very small change, but I needed +to shuffle a lot of code around in order to make it compile +(due to required define order...). + +Signed-off-by: Marius Tomaschewski +--- + runtime/msg.c | 232 +++++++++++++++++++++++++++++---------------------------- + 1 files changed, 117 insertions(+), 115 deletions(-) + +diff --git a/runtime/msg.c b/runtime/msg.c +index 409515a..8a1e66a 100644 +--- a/runtime/msg.c ++++ b/runtime/msg.c +@@ -287,6 +287,121 @@ static pthread_mutex_t mutTrimCtr; /* mutex to handle malloc trim */ + static int getAPPNAMELen(msg_t *pM, sbool bLockMutex); + + ++/* The following functions will support advanced output module ++ * multithreading, once this is implemented. Currently, we ++ * include them as hooks only. The idea is that we need to guard ++ * some msg objects data fields against concurrent access if ++ * we run on multiple threads. Please note that in any case this ++ * is not necessary for calls from INPUT modules, because they ++ * construct the message object and do this serially. Only when ++ * the message is in the processing queue, multiple threads may ++ * access a single object. Consequently, there are no guard functions ++ * for "set" methods, as these are called during input. Only "get" ++ * functions that modify important structures have them. ++ * rgerhards, 2007-07-20 ++ * We now support locked and non-locked operations, depending on ++ * the configuration of rsyslog. To support this, we use function ++ * pointers. Initially, we start in non-locked mode. There, all ++ * locking operations call into dummy functions. When locking is ++ * enabled, the function pointers are changed to functions doing ++ * actual work. We also introduced another MsgPrepareEnqueue() function ++ * which initializes the locking structures, if needed. This is ++ * necessary because internal messages during config file startup ++ * processing are always created in non-locking mode. So we can ++ * not initialize locking structures during constructions. We now ++ * postpone this until when the message is fully constructed and ++ * enqueued. Then we know the status of locking. This has a nice ++ * side effect, and that is that during the initial creation of ++ * the Msg object no locking needs to be done, which results in better ++ * performance. -- rgerhards, 2008-01-05 ++ */ ++static void (*funcLock)(msg_t *pMsg); ++static void (*funcUnlock)(msg_t *pMsg); ++static void (*funcDeleteMutex)(msg_t *pMsg); ++void (*funcMsgPrepareEnqueue)(msg_t *pMsg); ++#if 1 /* This is a debug aid */ ++#define MsgLock(pMsg) funcLock(pMsg) ++#define MsgUnlock(pMsg) funcUnlock(pMsg) ++#else ++#define MsgLock(pMsg) {dbgprintf("MsgLock line %d\n - ", __LINE__); funcLock(pMsg);; } ++#define MsgUnlock(pMsg) {dbgprintf("MsgUnlock line %d - ", __LINE__); funcUnlock(pMsg); } ++#endif ++ ++/* the next function is a dummy to be used by the looking functions ++ * when the class is not yet running in an environment where locking ++ * is necessary. Please note that the need to lock can (and will) change ++ * during a single run. Typically, this is depending on the operation mode ++ * of the message queues (which is operator-configurable). -- rgerhards, 2008-01-05 ++ */ ++static void MsgLockingDummy(msg_t __attribute__((unused)) *pMsg) ++{ ++ /* empty be design */ ++} ++ ++ ++/* The following function prepares a message for enqueue into the queue. This is ++ * where a message may be accessed by multiple threads. This implementation here ++ * is the version for multiple concurrent acces. It initializes the locking ++ * structures. ++ * TODO: change to an iRet interface! -- rgerhards, 2008-07-14 ++ */ ++static void MsgPrepareEnqueueLockingCase(msg_t *pThis) ++{ ++ BEGINfunc ++ assert(pThis != NULL); ++ pthread_mutex_init(&pThis->mut, NULL); ++ pThis->bDoLock = 1; ++ ENDfunc ++} ++ ++ ++/* ... and now the locking and unlocking implementations: */ ++static void MsgLockLockingCase(msg_t *pThis) ++{ ++ /* DEV debug only! dbgprintf("MsgLock(0x%lx)\n", (unsigned long) pThis); */ ++ assert(pThis != NULL); ++ if(pThis->bDoLock == 1) /* TODO: this is a testing hack, we should find a way with better performance! -- rgerhards, 2009-01-27 */ ++ pthread_mutex_lock(&pThis->mut); ++} ++ ++static void MsgUnlockLockingCase(msg_t *pThis) ++{ ++ /* DEV debug only! dbgprintf("MsgUnlock(0x%lx)\n", (unsigned long) pThis); */ ++ assert(pThis != NULL); ++ if(pThis->bDoLock == 1) /* TODO: this is a testing hack, we should find a way with better performance! -- rgerhards, 2009-01-27 */ ++ pthread_mutex_unlock(&pThis->mut); ++} ++ ++/* delete the mutex object on message destruction (locking case) ++ */ ++static void MsgDeleteMutexLockingCase(msg_t *pThis) ++{ ++ assert(pThis != NULL); ++ pthread_mutex_destroy(&pThis->mut); ++} ++ ++/* enable multiple concurrent access on the message object ++ * This works on a class-wide basis and can bot be undone. ++ * That is, if it is once enabled, it can not be disabled during ++ * the same run. When this function is called, no other thread ++ * must manipulate message objects. Then we would have race conditions, ++ * but guarding against this is counter-productive because it ++ * would cost additional time. Plus, it would be a programming error. ++ * rgerhards, 2008-01-05 ++ */ ++rsRetVal MsgEnableThreadSafety(void) ++{ ++ DEFiRet; ++ funcLock = MsgLockLockingCase; ++ funcUnlock = MsgUnlockLockingCase; ++ funcMsgPrepareEnqueue = MsgPrepareEnqueueLockingCase; ++ funcDeleteMutex = MsgDeleteMutexLockingCase; ++ RETiRet; ++} ++ ++/* end locking functions */ ++ ++ + static inline int getProtocolVersion(msg_t *pM) + { + return(pM->iProtocolVersion); +@@ -306,6 +421,7 @@ resolveDNS(msg_t *pMsg) { + uchar fromHostFQDN[NI_MAXHOST]; + DEFiRet; + ++ MsgLock(pMsg); + CHKiRet(objUse(net, CORE_COMPONENT)); + if(pMsg->msgFlags & NEEDS_DNSRESOL) { + localRet = net.cvthname(pMsg->rcvFrom.pfrominet, fromHost, fromHostFQDN, fromHostIP); +@@ -315,6 +431,7 @@ resolveDNS(msg_t *pMsg) { + } + } + finalize_it: ++ MsgUnlock(pMsg); + if(iRet != RS_RET_OK) { + /* best we can do: remove property */ + MsgSetRcvFromStr(pMsg, UCHAR_CONSTANT(""), 0, &propFromHost); +@@ -531,121 +648,6 @@ uchar *propIDToName(propid_t propID) + } + + +-/* The following functions will support advanced output module +- * multithreading, once this is implemented. Currently, we +- * include them as hooks only. The idea is that we need to guard +- * some msg objects data fields against concurrent access if +- * we run on multiple threads. Please note that in any case this +- * is not necessary for calls from INPUT modules, because they +- * construct the message object and do this serially. Only when +- * the message is in the processing queue, multiple threads may +- * access a single object. Consequently, there are no guard functions +- * for "set" methods, as these are called during input. Only "get" +- * functions that modify important structures have them. +- * rgerhards, 2007-07-20 +- * We now support locked and non-locked operations, depending on +- * the configuration of rsyslog. To support this, we use function +- * pointers. Initially, we start in non-locked mode. There, all +- * locking operations call into dummy functions. When locking is +- * enabled, the function pointers are changed to functions doing +- * actual work. We also introduced another MsgPrepareEnqueue() function +- * which initializes the locking structures, if needed. This is +- * necessary because internal messages during config file startup +- * processing are always created in non-locking mode. So we can +- * not initialize locking structures during constructions. We now +- * postpone this until when the message is fully constructed and +- * enqueued. Then we know the status of locking. This has a nice +- * side effect, and that is that during the initial creation of +- * the Msg object no locking needs to be done, which results in better +- * performance. -- rgerhards, 2008-01-05 +- */ +-static void (*funcLock)(msg_t *pMsg); +-static void (*funcUnlock)(msg_t *pMsg); +-static void (*funcDeleteMutex)(msg_t *pMsg); +-void (*funcMsgPrepareEnqueue)(msg_t *pMsg); +-#if 1 /* This is a debug aid */ +-#define MsgLock(pMsg) funcLock(pMsg) +-#define MsgUnlock(pMsg) funcUnlock(pMsg) +-#else +-#define MsgLock(pMsg) {dbgprintf("MsgLock line %d\n - ", __LINE__); funcLock(pMsg);; } +-#define MsgUnlock(pMsg) {dbgprintf("MsgUnlock line %d - ", __LINE__); funcUnlock(pMsg); } +-#endif +- +-/* the next function is a dummy to be used by the looking functions +- * when the class is not yet running in an environment where locking +- * is necessary. Please note that the need to lock can (and will) change +- * during a single run. Typically, this is depending on the operation mode +- * of the message queues (which is operator-configurable). -- rgerhards, 2008-01-05 +- */ +-static void MsgLockingDummy(msg_t __attribute__((unused)) *pMsg) +-{ +- /* empty be design */ +-} +- +- +-/* The following function prepares a message for enqueue into the queue. This is +- * where a message may be accessed by multiple threads. This implementation here +- * is the version for multiple concurrent acces. It initializes the locking +- * structures. +- * TODO: change to an iRet interface! -- rgerhards, 2008-07-14 +- */ +-static void MsgPrepareEnqueueLockingCase(msg_t *pThis) +-{ +- BEGINfunc +- assert(pThis != NULL); +- pthread_mutex_init(&pThis->mut, NULL); +- pThis->bDoLock = 1; +- ENDfunc +-} +- +- +-/* ... and now the locking and unlocking implementations: */ +-static void MsgLockLockingCase(msg_t *pThis) +-{ +- /* DEV debug only! dbgprintf("MsgLock(0x%lx)\n", (unsigned long) pThis); */ +- assert(pThis != NULL); +- if(pThis->bDoLock == 1) /* TODO: this is a testing hack, we should find a way with better performance! -- rgerhards, 2009-01-27 */ +- pthread_mutex_lock(&pThis->mut); +-} +- +-static void MsgUnlockLockingCase(msg_t *pThis) +-{ +- /* DEV debug only! dbgprintf("MsgUnlock(0x%lx)\n", (unsigned long) pThis); */ +- assert(pThis != NULL); +- if(pThis->bDoLock == 1) /* TODO: this is a testing hack, we should find a way with better performance! -- rgerhards, 2009-01-27 */ +- pthread_mutex_unlock(&pThis->mut); +-} +- +-/* delete the mutex object on message destruction (locking case) +- */ +-static void MsgDeleteMutexLockingCase(msg_t *pThis) +-{ +- assert(pThis != NULL); +- pthread_mutex_destroy(&pThis->mut); +-} +- +-/* enable multiple concurrent access on the message object +- * This works on a class-wide basis and can bot be undone. +- * That is, if it is once enabled, it can not be disabled during +- * the same run. When this function is called, no other thread +- * must manipulate message objects. Then we would have race conditions, +- * but guarding against this is counter-productive because it +- * would cost additional time. Plus, it would be a programming error. +- * rgerhards, 2008-01-05 +- */ +-rsRetVal MsgEnableThreadSafety(void) +-{ +- DEFiRet; +- funcLock = MsgLockLockingCase; +- funcUnlock = MsgUnlockLockingCase; +- funcMsgPrepareEnqueue = MsgPrepareEnqueueLockingCase; +- funcDeleteMutex = MsgDeleteMutexLockingCase; +- RETiRet; +-} +- +-/* end locking functions */ +- +- + /* This is common code for all Constructors. It is defined in an + * inline'able function so that we can save a function call in the + * actual constructors (otherwise, the msgConstruct would need +-- +1.7.3.4 + diff --git a/rsyslog.changes b/rsyslog.changes index f469edd..dd177c6 100644 --- a/rsyslog.changes +++ b/rsyslog.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Fri Apr 8 13:24:34 UTC 2011 - mt@suse.de + +- bugfix: race condition in deferred name resolution (id=238) + from v5.8.0 candidate. + ------------------------------------------------------------------- Thu Mar 24 16:34:31 UTC 2011 - mt@suse.de diff --git a/rsyslog.spec b/rsyslog.spec index f6f18dc..285b412 100644 --- a/rsyslog.spec +++ b/rsyslog.spec @@ -68,6 +68,7 @@ Source4: rsyslog.d.remote.conf.in %if 0%{?suse_version} >= 1140 Patch1: rsyslog-systemd-integration.bnc656104.diff %endif +Patch2: rsyslog-deferred-dns-query-race.diff %description Rsyslog is an enhanced multi-threaded syslogd supporting, among others, @@ -215,6 +216,7 @@ This module provides a UDP forwarder that allows changing the sender address. %setup -q -n %{name}-%{upstream_version} %if 0%{?suse_version} >= 1140 %patch1 -p1 +%patch2 -p1 # install the files systemd provides rather than what we provide. cp -a /usr/share/doc/packages/systemd/sd-daemon.[ch] runtime/ %endif