From bbd04fde309cb7e377726f8125d85110add26178 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Tue, 13 Sep 2011 17:13:49 +0200 Subject: [PATCH] NFQ: fix race condition at exit. A race condition was observed when leaving NFQ. This was caused by the queue handle being accessed after been nullified. This patch uses the handle mutex to protect the destruction and adds tests on nullity to avoid crashed. --- src/source-nfq.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/source-nfq.c b/src/source-nfq.c index 408b13255b..a49a97115a 100644 --- a/src/source-nfq.c +++ b/src/source-nfq.c @@ -514,13 +514,13 @@ TmEcode ReceiveNFQThreadDeinit(ThreadVars *t, void *data) } ntv->datalen = 0; - SCMutexLock(&nfq_init_lock); + SCMutexLock(&nq->mutex_qh); SCLogDebug("starting... will close queuenum %" PRIu32 "", nq->queue_num); if (nq->qh) { nfq_destroy_queue(nq->qh); nq->qh = NULL; } - SCMutexUnlock(&nfq_init_lock); + SCMutexUnlock(&nq->mutex_qh); return TM_ECODE_OK; } @@ -538,12 +538,12 @@ TmEcode VerdictNFQThreadDeinit(ThreadVars *tv, void *data) { NFQQueueVars *nq = NFQGetQueue(ntv->nfq_index); SCLogDebug("starting... will close queuenum %" PRIu32 "", nq->queue_num); - SCMutexLock(&nfq_init_lock); + SCMutexLock(&nq->mutex_qh); if (nq->qh) { nfq_destroy_queue(nq->qh); nq->qh = NULL; } - SCMutexUnlock(&nfq_init_lock); + SCMutexUnlock(&nq->mutex_qh); return TM_ECODE_OK; } @@ -681,7 +681,12 @@ void NFQRecvPkt(NFQQueueVars *t, NFQThreadVars *tv) { //printf("NFQRecvPkt: t %p, rv = %" PRId32 "\n", t, rv); SCMutexLock(&t->mutex_qh); - ret = nfq_handle_packet(t->h, tv->data, rv); + if (t->qh != NULL) { + ret = nfq_handle_packet(t->h, tv->data, rv); + } else { + SCLogWarning(SC_ERR_NFQ_HANDLE_PKT, "NFQ handle has been destroyed"); + ret = -1; + } SCMutexUnlock(&t->mutex_qh); if (ret != 0) { @@ -743,7 +748,12 @@ process_rv: //printf("NFQRecvPkt: t %p, rv = %" PRId32 "\n", t, rv); SCMutexLock(&t->mutex_qh); - ret = nfq_handle_packet(t->h, buf, rv); + if (t->qh) { + ret = nfq_handle_packet(t->h, buf, rv); + } else { + SCLogWarning(SC_ERR_NFQ_HANDLE_PKT, "NFQ handle has been destroyed"); + ret = -1; + } SCMutexUnlock(&t->mutex_qh); if (ret != 0) { @@ -810,6 +820,12 @@ TmEcode NFQSetVerdict(Packet *p) { //printf("%p verdicting on queue %" PRIu32 "\n", t, t->queue_num); SCMutexLock(&t->mutex_qh); + if (t->qh == NULL) { + /* Somebody has started a clean-up, we leave */ + SCMutexUnlock(&t->mutex_qh); + return TM_ECODE_OK; + } + if (p->action & ACTION_DROP) { verdict = NF_DROP; #ifdef COUNTERS