From 223c1e48610c27270fdb8532f5e4a920eb9874f3 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 2 Oct 2008 10:53:23 +0200 Subject: bugfix: rsyslogd could hang on HUP because getnameinfo() is not cancel-safe, but was not guarded against being cancelled. pthread_cancel() is routinely being called during HUP processing. --- ChangeLog | 4 ++++ net.c | 27 ++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index f16ac398..befba912 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,10 @@ Version 3.18.5 (rgerhards), 2008-10-?? - bugfix: imudp input module could cause segfault on HUP It did not properly de-init a variable acting as a linked list head. That resulted in trying to access freed memory blocks after the HUP. +- bugfix: rsyslogd could hang on HUP + because getnameinfo() is not cancel-safe, but was not guarded against + being cancelled. pthread_cancel() is routinely being called during + HUP processing. - doc bugfix: $ActionExecOnlyWhenPreviousIsSuspended was still misspelled as $...OnlyIfPrev... in some parts of the documentation. Thanks to Lorenzo M. Catucci for reporting this bug. diff --git a/net.c b/net.c index bbd6bec7..4054b863 100644 --- a/net.c +++ b/net.c @@ -106,6 +106,27 @@ static inline void MaskIP4 (struct in_addr *addr, uint8_t bits) { #define SIN(sa) ((struct sockaddr_in *)(sa)) #define SIN6(sa) ((struct sockaddr_in6 *)(sa)) + +/* This is a cancel-safe getnameinfo() version, because we learned + * (via drd/valgrind) that getnameinfo() seems to have some issues + * when being cancelled, at least if the module was dlloaded. + * rgerhards, 2008-09-30 + */ +static inline int +mygetnameinfo(const struct sockaddr *sa, socklen_t salen, + char *host, size_t hostlen, + char *serv, size_t servlen, int flags) +{ + int iCancelStateSave; + int i; + + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave); + i = getnameinfo(sa, salen, host, hostlen, serv, servlen, flags); + pthread_setcancelstate(iCancelStateSave, NULL); + return i; +} + + /* This function adds an allowed sender entry to the ACL linked list. * In any case, a single entry is added. If an error occurs, the * function does its error reporting itself. All validity checks @@ -368,7 +389,7 @@ void PrintAllowedSenders(int iListToPrint) if (F_ISSET(pSender->allowedSender.flags, ADDR_NAME)) dbgprintf ("\t%s\n", pSender->allowedSender.addr.HostWildcard); else { - if(getnameinfo (pSender->allowedSender.addr.NetAddr, + if(mygetnameinfo (pSender->allowedSender.addr.NetAddr, SALEN(pSender->allowedSender.addr.NetAddr), (char*)szIP, 64, NULL, 0, NI_NUMERICHOST) == 0) { dbgprintf ("\t%s/%u\n", szIP, pSender->SignificantBits); @@ -642,7 +663,7 @@ gethname(struct sockaddr_storage *f, uchar *pszHostFQDN) assert(f != NULL); assert(pszHostFQDN != NULL); - error = getnameinfo((struct sockaddr *)f, SALEN((struct sockaddr *)f), + error = mygetnameinfo((struct sockaddr *)f, SALEN((struct sockaddr *)f), ip, sizeof ip, NULL, 0, NI_NUMERICHOST); if (error) { @@ -656,7 +677,7 @@ gethname(struct sockaddr_storage *f, uchar *pszHostFQDN) sigaddset(&nmask, SIGHUP); pthread_sigmask(SIG_BLOCK, &nmask, &omask); - error = getnameinfo((struct sockaddr *)f, SALEN((struct sockaddr *) f), + error = mygetnameinfo((struct sockaddr *)f, SALEN((struct sockaddr *) f), (char*)pszHostFQDN, NI_MAXHOST, NULL, 0, NI_NAMEREQD); if (error == 0) { -- cgit