summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRainer Gerhards <rgerhards@adiscon.com>2008-03-13 11:04:33 +0000
committerRainer Gerhards <rgerhards@adiscon.com>2008-03-13 11:04:33 +0000
commit17f36a76cbf15e088d2973ed5608f93e09827f8a (patch)
treeb6c13e0b6552821b39770011e161d46f8b647065
parent49c52d090aea8dc898cc855278b1054daff1461f (diff)
downloadrsyslog-17f36a76cbf15e088d2973ed5608f93e09827f8a.tar.gz
rsyslog-17f36a76cbf15e088d2973ed5608f93e09827f8a.tar.xz
rsyslog-17f36a76cbf15e088d2973ed5608f93e09827f8a.zip
bugfix: imgssapi segfaulted under some conditions; this fix is actually not
just a fix but a change in the object model. Thanks to varmojfekoj for providing the bug report, an initial fix and lots of good discussion that lead to where we finally ended up.
-rw-r--r--ChangeLog4
-rw-r--r--modules.c5
-rw-r--r--plugins/imgssapi/imgssapi.c37
-rw-r--r--plugins/imtcp/imtcp.c10
-rw-r--r--tcps_sess.c11
-rw-r--r--tcpsrv.c35
-rw-r--r--tcpsrv.h4
7 files changed, 47 insertions, 59 deletions
diff --git a/ChangeLog b/ChangeLog
index 9d196446..5351cabc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -11,6 +11,10 @@ Version 3.12.2 (rgerhards), 2008-03-??
to Johnny Tan for an excellent bug report
- implemented dynamic module unload capability (not visible to end user)
- some more internal cleanup
+- bugfix: imgssapi segfaulted under some conditions; this fix is actually
+ not just a fix but a change in the object model. Thanks to varmojfekoj
+ for providing the bug report, an initial fix and lots of good discussion
+ that lead to where we finally ended up.
---------------------------------------------------------------------------
Version 3.12.1 (rgerhards), 2008-03-06
- added library plugins, which can be automatically loaded
diff --git a/modules.c b/modules.c
index 70945cae..d5032772 100644
--- a/modules.c
+++ b/modules.c
@@ -484,12 +484,7 @@ modUnlinkAndDestroy(modInfo_t *pThis)
/* finally, we are ready for the module to go away... */
dbgprintf("Unloading module %s\n", modGetName(pThis));
CHKiRet(modPrepareUnload(pThis));
-modInfo_t *prev, *next;
-char *name = strdup(pThis->pszName);
-prev = pThis->pPrev; next = pThis->pNext;
moduleDestruct(pThis);
-dbgprintf("end unload, pThis %p (%s), prev %p, next %p\n", pThis, name, prev, next);
-free(name);
finalize_it:
RETiRet;
diff --git a/plugins/imgssapi/imgssapi.c b/plugins/imgssapi/imgssapi.c
index f551d616..8d406a7e 100644
--- a/plugins/imgssapi/imgssapi.c
+++ b/plugins/imgssapi/imgssapi.c
@@ -68,8 +68,8 @@ static rsRetVal addGSSListener(void __attribute__((unused)) *pVal, uchar *pNewVa
static int TCPSessGSSInit(void);
static void TCPSessGSSClose(tcps_sess_t* pSess);
static int TCPSessGSSRecv(tcps_sess_t *pSess, void *buf, size_t buf_len);
-static rsRetVal onSessAccept(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd);
-static rsRetVal OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd);
+static rsRetVal onSessAccept(tcpsrv_t *pThis, tcps_sess_t *ppSess);
+static rsRetVal OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t *ppSess);
/* static data */
DEF_IMOD_STATIC_DATA
@@ -184,7 +184,7 @@ isPermittedHost(struct sockaddr *addr, char *fromHostFQDN, void *pUsrSrv, void*p
}
static rsRetVal
-onSessAccept(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
+onSessAccept(tcpsrv_t *pThis, tcps_sess_t *pSess)
{
DEFiRet;
gsssrv_t *pGSrv;
@@ -192,10 +192,8 @@ onSessAccept(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
pGSrv = (gsssrv_t*) pThis->pUsr;
if(pGSrv->allowedMethods & ALLOWEDMETHOD_GSS) {
- iRet = OnSessAcceptGSS(pThis, ppSess, fd);
- } else {
- iRet = tcpsrv.SessAccept(pThis, ppSess, fd);
- }
+ iRet = OnSessAcceptGSS(pThis, pSess);
+ }
RETiRet;
}
@@ -246,7 +244,7 @@ onErrClose(tcps_sess_t *pSess)
static int*
doOpenLstnSocks(tcpsrv_t *pSrv)
{
- int *pRet;
+ int *pRet = NULL;
gsssrv_t *pGSrv;
ISOBJ_TYPE_assert(pSrv, tcpsrv);
@@ -363,11 +361,10 @@ static int TCPSessGSSInit(void)
/* returns 0 if all went OK, -1 if it failed
- * Calls tcpsrv's SessAccept() and then tries to guess if the connection uses
- * gssapi.
+ * tries to guess if the connection uses gssapi.
*/
static rsRetVal
-OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
+OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t *pSess)
{
DEFiRet;
gss_buffer_desc send_tok, recv_tok;
@@ -378,13 +375,9 @@ OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
int fdSess;
char allowedMethods;
gsssrv_t *pGSrv;
- tcps_sess_t *pSess;
gss_sess_t *pGSess;
- assert(ppSess != NULL);
-
- /* first do the usual coding */
- CHKiRet(tcpsrv.SessAccept(pThis, &pSess, fd));
+ assert(pSess != NULL);
pGSrv = (gsssrv_t*) pThis->pUsr;
pGSess = (gss_sess_t*) pSess->pUsr;
@@ -413,7 +406,6 @@ OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
} while (ret < 0 && errno == EINTR);
if (ret < 0) {
errmsg.LogError(NO_ERRCODE, "TCP session %p will be closed, error ignored\n", pSess);
- tcps_sess.Close(pSess);
ABORT_FINALIZE(RS_RET_ERR); // TODO: define good error codes
} else if (ret == 0) {
dbgprintf("GSS-API Reverting to plain TCP\n");
@@ -429,7 +421,6 @@ OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
dbgprintf("GSS-API Connection closed by peer\n");
else
errmsg.LogError(NO_ERRCODE, "TCP(GSS) session %p will be closed, error ignored\n", pSess);
- tcps_sess.Close(pSess);
ABORT_FINALIZE(RS_RET_ERR); // TODO: define good error codes
}
@@ -450,7 +441,6 @@ OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
dbgprintf("GSS-API Connection closed by peer\n");
else
errmsg.LogError(NO_ERRCODE, "TCP session %p will be closed, error ignored\n", pSess);
- tcps_sess.Close(pSess);
ABORT_FINALIZE(RS_RET_ERR); // TODO: define good error codes
}
}
@@ -473,7 +463,6 @@ OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
do {
if (gssutil.recv_token(fdSess, &recv_tok) <= 0) {
errmsg.LogError(NO_ERRCODE, "TCP session %p will be closed, error ignored\n", pSess);
- tcps_sess.Close(pSess);
ABORT_FINALIZE(RS_RET_ERR); // TODO: define good error codes
}
maj_stat = gss_accept_sec_context(&acc_sec_min_stat, context, gss_server_creds,
@@ -493,16 +482,13 @@ OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
dbgprintf("tcp session socket with new data: #%d\n", fdSess);
if(tcps_sess.DataRcvd(pSess, buf, ret) == 0) {
errmsg.LogError(NO_ERRCODE, "Tearing down TCP Session %p - see "
- "previous messages for reason(s)\n",
- pSess);
- tcps_sess.Close(pSess);
+ "previous messages for reason(s)\n", pSess);
ABORT_FINALIZE(RS_RET_ERR); // TODO: define good error codes
}
pGSess->allowedMethods = ALLOWEDMETHOD_TCP;
ABORT_FINALIZE(RS_RET_OK); // TODO: define good error codes
}
gssutil.display_status("accepting context", maj_stat, acc_sec_min_stat);
- tcps_sess.Close(pSess);
ABORT_FINALIZE(RS_RET_ERR); // TODO: define good error codes
}
if (send_tok.length != 0) {
@@ -511,7 +497,6 @@ OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
errmsg.LogError(NO_ERRCODE, "TCP session %p will be closed, error ignored\n", pSess);
if (*context != GSS_C_NO_CONTEXT)
gss_delete_sec_context(&min_stat, context, GSS_C_NO_BUFFER);
- tcps_sess.Close(pSess);
ABORT_FINALIZE(RS_RET_ERR); // TODO: define good error codes
}
gss_release_buffer(&min_stat, &send_tok);
@@ -531,8 +516,6 @@ OnSessAcceptGSS(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
pGSess->allowedMethods = ALLOWEDMETHOD_GSS;
}
- *ppSess = pSess;
-
finalize_it:
RETiRet;
}
diff --git a/plugins/imtcp/imtcp.c b/plugins/imtcp/imtcp.c
index ed0131b8..7baa95f2 100644
--- a/plugins/imtcp/imtcp.c
+++ b/plugins/imtcp/imtcp.c
@@ -70,15 +70,6 @@ isPermittedHost(struct sockaddr *addr, char *fromHostFQDN, void __attribute__((u
}
-static rsRetVal
-onSessAccept(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
-{
- DEFiRet;
- iRet = tcpsrv.SessAccept(pThis, ppSess, fd);
- RETiRet;
-}
-
-
static int*
doOpenLstnSocks(tcpsrv_t *pSrv)
{
@@ -132,7 +123,6 @@ static rsRetVal addTCPListener(void __attribute__((unused)) *pVal, uchar *pNewVa
CHKiRet(tcpsrv.SetCBIsPermittedHost(pOurTcpsrv, isPermittedHost));
CHKiRet(tcpsrv.SetCBRcvData(pOurTcpsrv, doRcvData));
CHKiRet(tcpsrv.SetCBOpenLstnSocks(pOurTcpsrv, doOpenLstnSocks));
- CHKiRet(tcpsrv.SetCBOnSessAccept(pOurTcpsrv, onSessAccept));
CHKiRet(tcpsrv.SetCBOnRegularClose(pOurTcpsrv, onRegularClose));
CHKiRet(tcpsrv.SetCBOnErrClose(pOurTcpsrv, onErrClose));
tcpsrv.configureTCPListen(pOurTcpsrv, (char *) pNewVal);
diff --git a/tcps_sess.c b/tcps_sess.c
index 74de3c28..c8ac3a37 100644
--- a/tcps_sess.c
+++ b/tcps_sess.c
@@ -57,6 +57,10 @@
DEFobjStaticHelpers
DEFobjCurrIf(errmsg)
+/* forward definitions */
+static rsRetVal Close(tcps_sess_t *pThis);
+
+
/* Standard-Constructor
*/
BEGINobjConstruct(tcps_sess) /* be sure to specify the object type also in END macro! */
@@ -86,6 +90,9 @@ finalize_it:
/* destructor for the tcps_sess object */
BEGINobjDestruct(tcps_sess) /* be sure to specify the object type also in END and CODESTART macros! */
CODESTARTobjDestruct(tcps_sess)
+ if(pThis->sock != -1)
+ Close(pThis);
+
if(pThis->pSrv->pOnSessDestruct != NULL) {
pThis->pSrv->pOnSessDestruct(&pThis->pUsr);
}
@@ -211,8 +218,8 @@ finalize_it:
}
-/* Closes a TCP session and marks its slot in the session
- * table as unused. No attention is paid to the return code
+/* Closes a TCP session
+ * No attention is paid to the return code
* of close, so potential-double closes are not detected.
*/
static rsRetVal
diff --git a/tcpsrv.c b/tcpsrv.c
index 0f87165d..4030b206 100644
--- a/tcpsrv.c
+++ b/tcpsrv.c
@@ -422,13 +422,16 @@ static int *create_tcp_socket(tcpsrv_t *pThis)
* connection is immediately dropped.
* ppSess has a pointer to the newly created session, if it succeds.
* If it does not succeed, no session is created and ppSess is
- * undefined. -- rgerhards, 2008-03-02
+ * undefined. If the user has provided an OnSessAccept Callback,
+ * this one is executed immediately after creation of the
+ * session object, so that it can do its own initializatin.
+ * rgerhards, 2008-03-02
*/
static rsRetVal
SessAccept(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
{
DEFiRet;
-
+ tcps_sess_t *pSess;
int newConn;
int iSess = -1;
struct sockaddr_storage addr;
@@ -455,13 +458,10 @@ SessAccept(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
//was: return -1;
} else {
/* we found a free spot and can construct our session object */
- CHKiRet(tcps_sess.Construct(ppSess));
- CHKiRet(tcps_sess.SetTcpsrv(*ppSess, pThis));
+ CHKiRet(tcps_sess.Construct(&pSess));
+ CHKiRet(tcps_sess.SetTcpsrv(pSess, pThis));
}
-
- pThis->pSessions[iSess] = *ppSess;
-
/* OK, we have a "good" index... */
/* get the host name */
if(net.cvthname(&addr, fromHost, fromHostFQDN) != RS_RET_OK) {
@@ -494,10 +494,18 @@ SessAccept(tcpsrv_t *pThis, tcps_sess_t **ppSess, int fd)
/* OK, we have an allowed sender, so let's continue, what
* means we can finally fill in the session object.
*/
- CHKiRet(tcps_sess.SetHost(pThis->pSessions[iSess], fromHost));
- CHKiRet(tcps_sess.SetSock(pThis->pSessions[iSess], newConn));
- CHKiRet(tcps_sess.SetMsgIdx(pThis->pSessions[iSess], 0));
- CHKiRet(tcps_sess.ConstructFinalize(pThis->pSessions[iSess]));
+ CHKiRet(tcps_sess.SetHost(pSess, fromHost));
+ CHKiRet(tcps_sess.SetSock(pSess, newConn));
+ CHKiRet(tcps_sess.SetMsgIdx(pSess, 0));
+ CHKiRet(tcps_sess.ConstructFinalize(pSess));
+
+ /* check if we need to call our callback */
+ if(pThis->pOnSessAccept != NULL) {
+ CHKiRet(pThis->pOnSessAccept(pThis, pSess));
+ }
+
+ *ppSess = pSess;
+ pThis->pSessions[iSess] = pSess;
finalize_it:
if(iRet != RS_RET_OK) {
@@ -578,7 +586,8 @@ Run(tcpsrv_t *pThis)
for (i = 0; i < *pThis->pSocksLstn; i++) {
if (FD_ISSET(pThis->pSocksLstn[i+1], &readfds)) {
dbgprintf("New connect on TCP inetd socket: #%d\n", pThis->pSocksLstn[i+1]);
- pThis->pOnSessAccept(pThis, &pNewSess, pThis->pSocksLstn[i+1]);
+ //pThis->pOnSessAccept(pThis, &pNewSess, pThis->pSocksLstn[i+1]);
+ SessAccept(pThis, &pNewSess, pThis->pSocksLstn[i+1]);
--nfds; /* indicate we have processed one */
}
}
@@ -687,7 +696,7 @@ SetCBOnListenDeinit(tcpsrv_t *pThis, int (*pCB)(void*))
}
static rsRetVal
-SetCBOnSessAccept(tcpsrv_t *pThis, rsRetVal (*pCB)(tcpsrv_t*, tcps_sess_t**, int))
+SetCBOnSessAccept(tcpsrv_t *pThis, rsRetVal (*pCB)(tcpsrv_t*, tcps_sess_t*))
{
DEFiRet;
pThis->pOnSessAccept = pCB;
diff --git a/tcpsrv.h b/tcpsrv.h
index fb9f4d21..20a7ea25 100644
--- a/tcpsrv.h
+++ b/tcpsrv.h
@@ -42,7 +42,7 @@ typedef struct tcpsrv_s {
rsRetVal (*pOnRegularClose)(tcps_sess_t *pSess);
rsRetVal (*pOnErrClose)(tcps_sess_t *pSess);
/* session specific callbacks */
- rsRetVal (*pOnSessAccept)(struct tcpsrv_s *, tcps_sess_t**, int fd);
+ rsRetVal (*pOnSessAccept)(struct tcpsrv_s *, tcps_sess_t*);
rsRetVal (*OnSessConstructFinalize)(void*);
rsRetVal (*pOnSessDestruct)(void*);
} tcpsrv_t;
@@ -68,7 +68,7 @@ BEGINinterface(tcpsrv) /* name must also be changed in ENDinterface macro! */
rsRetVal (*SetCBOnRegularClose)(tcpsrv_t*, rsRetVal (*) (tcps_sess_t*));
rsRetVal (*SetCBOnErrClose)(tcpsrv_t*, rsRetVal (*) (tcps_sess_t*));
/* session specifics */
- rsRetVal (*SetCBOnSessAccept)(tcpsrv_t*, rsRetVal (*) (tcpsrv_t*, tcps_sess_t**, int));
+ rsRetVal (*SetCBOnSessAccept)(tcpsrv_t*, rsRetVal (*) (tcpsrv_t*, tcps_sess_t*));
rsRetVal (*SetCBOnSessDestruct)(tcpsrv_t*, rsRetVal (*) (void*));
rsRetVal (*SetCBOnSessConstructFinalize)(tcpsrv_t*, rsRetVal (*) (void*));
ENDinterface(tcpsrv)