detect: simplify flow locking

To simplify locking, move all locking out of the individual detect
code. Instead at the start of detection lock the flow, and at the
end of detection unlock it.

The lua code can be called without a lock still (from the output
code paths), so still pass around a lock hint to take care of this.
pull/2089/head
Victor Julien 10 years ago
parent 6f560144c1
commit 408948815f

@ -267,8 +267,7 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
if (p->flow != NULL) {
/* Update flow flags for iponly */
FLOWLOCK_WRLOCK(p->flow);
FlowSetIPOnlyFlagNoLock(p->flow, p->flowflags & FLOW_PKT_TOSERVER ? 1 : 0);
FlowSetIPOnlyFlag(p->flow, (p->flowflags & FLOW_PKT_TOSERVER) ? 1 : 0);
if (s->action & ACTION_DROP)
p->flow->flags |= FLOW_ACTION_DROP;
@ -281,7 +280,6 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
if (s->action & ACTION_PASS) {
FlowSetNoPacketInspectionFlag(p->flow);
}
FLOWLOCK_UNLOCK(p->flow);
}
}
}
@ -299,7 +297,7 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
(p->alerts.alerts[i].flags &
(PACKET_ALERT_FLAG_STATE_MATCH|PACKET_ALERT_FLAG_STREAM_MATCH)))
{
FlowLockSetNoPacketInspectionFlag(p->flow);
FlowSetNoPacketInspectionFlag(p->flow);
}
break;
@ -310,10 +308,8 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
(s->flags & SIG_FLAG_APPLAYER))
&& p->flow != NULL)
{
FLOWLOCK_WRLOCK(p->flow);
/* This will apply only on IPS mode (check StreamTcpPacket) */
p->flow->flags |= FLOW_ACTION_DROP;
FLOWLOCK_UNLOCK(p->flow);
p->flow->flags |= FLOW_ACTION_DROP; // XXX API?
}
}

@ -536,13 +536,9 @@ int DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThreadCtx
}
else if (sm->type == DETECT_LUA) {
SCLogDebug("lua starting");
/* for flowvar gets and sets we need to know the flow's lock status */
int flow_lock = LUA_FLOW_LOCKED_BY_PARENT;
if (inspection_mode <= DETECT_ENGINE_CONTENT_INSPECTION_MODE_STREAM)
flow_lock = LUA_FLOW_NOT_LOCKED_BY_PARENT;
if (DetectLuaMatchBuffer(det_ctx, s, sm, buffer, buffer_len,
det_ctx->buffer_offset, f, flow_lock) != 1)
det_ctx->buffer_offset, f) != 1)
{
SCLogDebug("lua no_match");
goto no_match;

@ -378,8 +378,6 @@ int DeStateFlowHasInspectableState(Flow *f, AppProto alproto,
{
int r = 0;
FLOWLOCK_WRLOCK(f);
if (!(flags & STREAM_EOF) && f->de_state &&
f->detect_alversion[flags & STREAM_TOSERVER ? 0 : 1] == alversion) {
SCLogDebug("unchanged state");
@ -389,8 +387,6 @@ int DeStateFlowHasInspectableState(Flow *f, AppProto alproto,
} else {
r = 0;
}
FLOWLOCK_UNLOCK(f);
return r;
}
@ -490,7 +486,6 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
SCLogDebug("rule %u", s->id);
FLOWLOCK_WRLOCK(f);
/* TX based matches (inspect engines) */
if (AppLayerParserProtocolSupportsTxs(f->proto, alproto)) {
uint64_t tx_id = 0;
@ -713,8 +708,6 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
}
end:
FLOWLOCK_UNLOCK(f);
det_ctx->tx_id = 0;
det_ctx->tx_id_set = 0;
return alert_cnt ? 1:0;
@ -907,9 +900,7 @@ static int DoInspectItem(ThreadVars *tv,
RULE_PROFILING_END(det_ctx, s, (alert == 1), p);
if (alert) {
det_ctx->flow_locked = 1;
SigMatchSignaturesRunPostMatch(tv, de_ctx, det_ctx, p, s);
det_ctx->flow_locked = 0;
if (!(s->flags & SIG_FLAG_NOALERT)) {
PacketAlertAppend(det_ctx, s, p, inspect_tx_id,
@ -1000,9 +991,7 @@ static int DoInspectFlowRule(ThreadVars *tv,
item->nm = sm;
if (alert) {
det_ctx->flow_locked = 1;
SigMatchSignaturesRunPostMatch(tv, de_ctx, det_ctx, p, s);
det_ctx->flow_locked = 0;
if (!(s->flags & SIG_FLAG_NOALERT)) {
PacketAlertAppend(det_ctx, s, p, 0,
@ -1028,14 +1017,11 @@ void DeStateDetectContinueDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
uint64_t total_txs = 0;
uint8_t direction = (flags & STREAM_TOSERVER) ? 0 : 1;
FLOWLOCK_WRLOCK(f);
SCLogDebug("starting continue detection for packet %"PRIu64, p->pcap_cnt);
if (AppLayerParserProtocolSupportsTxs(f->proto, alproto)) {
void *alstate = FlowGetAppState(f);
if (!StateIsValid(alproto, alstate)) {
FLOWLOCK_UNLOCK(f);
return;
}
@ -1134,7 +1120,6 @@ void DeStateDetectContinueDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
}
end:
FLOWLOCK_UNLOCK(f);
det_ctx->tx_id = 0;
det_ctx->tx_id_set = 0;
return;
@ -1147,13 +1132,10 @@ end:
* \note it is possible that f->alstate, f->alparser are NULL */
void DeStateUpdateInspectTransactionId(Flow *f, const uint8_t flags)
{
FLOWLOCK_WRLOCK(f);
if (f->alparser && f->alstate) {
AppLayerParserSetTransactionInspectId(f->alparser, f->proto,
f->alproto, f->alstate, flags);
}
FLOWLOCK_UNLOCK(f);
return;
}

@ -132,7 +132,6 @@ int TagFlowAdd(Packet *p, DetectTagDataEntry *tde)
if (p->flow == NULL)
return 1;
FLOWLOCK_WRLOCK(p->flow);
iter = FlowGetStorageById(p->flow, flow_tag_id);
if (iter != NULL) {
/* First iterate installed entries searching a duplicated sid/gid */
@ -169,7 +168,6 @@ int TagFlowAdd(Packet *p, DetectTagDataEntry *tde)
SCLogDebug("Max tags for sessions reached (%"PRIu16")", tag_cnt);
}
FLOWLOCK_UNLOCK(p->flow);
return updated;
}
@ -516,9 +514,7 @@ void TagHandlePacket(DetectEngineCtx *de_ctx,
/* First update and get session tags */
if (p->flow != NULL) {
FLOWLOCK_WRLOCK(p->flow);
TagHandlePacketFlow(p->flow, p);
FLOWLOCK_UNLOCK(p->flow);
}
Host *src = HostLookupHostFromHash(&p->src);

@ -203,9 +203,6 @@ int DetectFilestorePostMatch(ThreadVars *t, DetectEngineThreadCtx *det_ctx, Pack
else
flags |= STREAM_TOSERVER;
if (det_ctx->flow_locked == 0)
FLOWLOCK_WRLOCK(p->flow);
FileContainer *ffc = AppLayerParserGetFiles(p->flow->proto, p->flow->alproto,
p->flow->alstate, flags);
@ -225,9 +222,6 @@ int DetectFilestorePostMatch(ThreadVars *t, DetectEngineThreadCtx *det_ctx, Pack
}
}
if (det_ctx->flow_locked == 0)
FLOWLOCK_UNLOCK(p->flow);
SCReturnInt(0);
}

@ -89,16 +89,12 @@ void DetectFlowintRegister(void)
int DetectFlowintMatch(ThreadVars *t, DetectEngineThreadCtx *det_ctx,
Packet *p, Signature *s, const SigMatchCtx *ctx)
{
const int flow_locked = det_ctx->flow_locked;
const DetectFlowintData *sfd = (const DetectFlowintData *)ctx;
FlowVar *fv;
FlowVar *fvt;
uint32_t targetval;
int ret = 0;
if (flow_locked == 0)
FLOWLOCK_WRLOCK(p->flow);
/** ATM If we are going to compare the current var with another
* that doesn't exist, the default value will be zero;
* if you don't want this behaviour, you can use the keyword
@ -210,8 +206,6 @@ int DetectFlowintMatch(ThreadVars *t, DetectEngineThreadCtx *det_ctx,
}
end:
if (flow_locked == 0)
FLOWLOCK_UNLOCK(p->flow);
return ret;
}

@ -98,9 +98,6 @@ int DetectFlowvarMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx, Packet *p
int ret = 0;
DetectFlowvarData *fd = (DetectFlowvarData *)ctx;
/* we need a lock */
FLOWLOCK_RDLOCK(p->flow);
FlowVar *fv = FlowVarGet(p->flow, fd->idx);
if (fv != NULL) {
uint8_t *ptr = SpmSearch(fv->data.fv_str.value,
@ -109,7 +106,6 @@ int DetectFlowvarMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx, Packet *p
if (ptr != NULL)
ret = 1;
}
FLOWLOCK_UNLOCK(p->flow);
return ret;
}

@ -293,7 +293,7 @@ void LuaDumpStack(lua_State *state)
int DetectLuaMatchBuffer(DetectEngineThreadCtx *det_ctx, Signature *s, SigMatch *sm,
uint8_t *buffer, uint32_t buffer_len, uint32_t offset,
Flow *f, int flow_lock)
Flow *f)
{
SCEnter();
int ret = 0;
@ -310,6 +310,10 @@ int DetectLuaMatchBuffer(DetectEngineThreadCtx *det_ctx, Signature *s, SigMatch
SCReturnInt(0);
/* setup extension data for use in lua c functions */
int flow_lock = (f != NULL) ? /* if we have a flow, it's locked */
LUA_FLOW_LOCKED_BY_PARENT :
LUA_FLOW_NOT_LOCKED_BY_PARENT;
LuaExtensionsMatchSetup(tluajit->luastate, luajit, det_ctx,
f, flow_lock, /* no packet in the ctx */NULL, 0);
@ -419,8 +423,13 @@ static int DetectLuaMatch (ThreadVars *tv, DetectEngineThreadCtx *det_ctx,
flags = STREAM_TOCLIENT;
LuaStateSetThreadVars(tluajit->luastate, tv);
int flow_lock = (p->flow != NULL) ? /* if we have a flow, it's locked */
LUA_FLOW_LOCKED_BY_PARENT :
LUA_FLOW_NOT_LOCKED_BY_PARENT;
LuaExtensionsMatchSetup(tluajit->luastate, luajit, det_ctx,
p->flow, /* flow not locked */LUA_FLOW_NOT_LOCKED_BY_PARENT, p, flags);
p->flow, flow_lock, p, flags);
if ((tluajit->flags & DATATYPE_PAYLOAD) && p->payload_len == 0)
SCReturnInt(0);
@ -430,10 +439,7 @@ static int DetectLuaMatch (ThreadVars *tv, DetectEngineThreadCtx *det_ctx,
if (p->flow == NULL)
SCReturnInt(0);
FLOWLOCK_RDLOCK(p->flow);
int alproto = p->flow->alproto;
FLOWLOCK_UNLOCK(p->flow);
AppProto alproto = p->flow->alproto;
if (tluajit->alproto != alproto)
SCReturnInt(0);
}
@ -452,7 +458,6 @@ static int DetectLuaMatch (ThreadVars *tv, DetectEngineThreadCtx *det_ctx,
lua_settable(tluajit->luastate, -3);
}
if (tluajit->alproto == ALPROTO_HTTP) {
FLOWLOCK_RDLOCK(p->flow);
HtpState *htp_state = p->flow->alstate;
if (htp_state != NULL && htp_state->connp != NULL) {
htp_tx_t *tx = NULL;
@ -474,7 +479,6 @@ static int DetectLuaMatch (ThreadVars *tv, DetectEngineThreadCtx *det_ctx,
}
}
}
FLOWLOCK_UNLOCK(p->flow);
}
int retval = lua_pcall(tluajit->luastate, 1, 1, 0);

@ -44,7 +44,7 @@ typedef struct DetectLuaData {
int negated;
char *filename;
uint32_t flags;
int alproto;
AppProto alproto;
char *buffername; /* buffer name in case of a single buffer */
uint16_t flowint[DETECT_LUAJIT_MAX_FLOWINTS];
uint16_t flowints;
@ -61,7 +61,7 @@ typedef struct DetectLuaData {
void DetectLuaRegister (void);
int DetectLuaMatchBuffer(DetectEngineThreadCtx *det_ctx, Signature *s, SigMatch *sm,
uint8_t *buffer, uint32_t buffer_len, uint32_t offset,
Flow *f, int flow_lock);
Flow *f);
#ifdef HAVE_LUAJIT
int DetectLuajitSetupStatesPool(int num, int reloads);

@ -907,18 +907,15 @@ static inline void DetectMpmPrefilter(DetectEngineCtx *de_ctx,
/* all http based mpms */
if (has_state && alproto == ALPROTO_HTTP) {
FLOWLOCK_WRLOCK(p->flow);
void *alstate = FlowGetAppState(p->flow);
if (alstate == NULL) {
SCLogDebug("no alstate");
FLOWLOCK_UNLOCK(p->flow);
return;
}
HtpState *htp_state = (HtpState *)alstate;
if (htp_state->connp == NULL) {
SCLogDebug("no HTTP connp");
FLOWLOCK_UNLOCK(p->flow);
return;
}
@ -1040,18 +1037,14 @@ static inline void DetectMpmPrefilter(DetectEngineCtx *de_ctx,
}
}
} /* for */
FLOWLOCK_UNLOCK(p->flow);
}
/* all dns based mpms */
else if (alproto == ALPROTO_DNS && has_state) {
if (p->flowflags & FLOW_PKT_TOSERVER) {
if (det_ctx->sgh->flags & SIG_GROUP_HEAD_MPM_DNSQUERY) {
FLOWLOCK_RDLOCK(p->flow);
void *alstate = FlowGetAppState(p->flow);
if (alstate == NULL) {
SCLogDebug("no alstate");
FLOWLOCK_UNLOCK(p->flow);
return;
}
@ -1066,35 +1059,28 @@ static inline void DetectMpmPrefilter(DetectEngineCtx *de_ctx,
DetectDnsQueryInspectMpm(det_ctx, p->flow, alstate, flags, tx, idx);
PACKET_PROFILING_DETECT_END(p, PROF_DETECT_MPM_DNSQUERY);
}
FLOWLOCK_UNLOCK(p->flow);
}
}
} else if (alproto == ALPROTO_TLS && has_state) {
if (p->flowflags & FLOW_PKT_TOSERVER) {
if (det_ctx->sgh->flags & SIG_GROUP_HEAD_MPM_TLSSNI) {
FLOWLOCK_RDLOCK(p->flow);
void *alstate = FlowGetAppState(p->flow);
if (alstate == NULL) {
SCLogDebug("no alstate");
FLOWLOCK_UNLOCK(p->flow);
return;
}
PACKET_PROFILING_DETECT_START(p, PROF_DETECT_MPM_TLSSNI);
DetectTlsSniInspectMpm(det_ctx, p->flow, alstate, flags);
PACKET_PROFILING_DETECT_END(p, PROF_DETECT_MPM_TLSSNI);
FLOWLOCK_UNLOCK(p->flow);
}
}
} else if (alproto == ALPROTO_SMTP && has_state) {
if (p->flowflags & FLOW_PKT_TOSERVER) {
if (det_ctx->sgh->flags & SIG_GROUP_HEAD_MPM_FD_SMTP) {
FLOWLOCK_RDLOCK(p->flow);
void *alstate = FlowGetAppState(p->flow);
if (alstate == NULL) {
SCLogDebug("no alstate");
FLOWLOCK_UNLOCK(p->flow);
return;
}
@ -1110,7 +1096,6 @@ static inline void DetectMpmPrefilter(DetectEngineCtx *de_ctx,
DetectEngineRunSMTPMpm(de_ctx, det_ctx, p->flow, smtp_state, flags, tx, idx);
PACKET_PROFILING_DETECT_END(p, PROF_DETECT_MPM_FD_SMTP);
}
FLOWLOCK_UNLOCK(p->flow);
}
}
}
@ -1155,11 +1140,9 @@ static inline void DetectMpmPrefilter(DetectEngineCtx *de_ctx,
if (p->flowflags & FLOW_PKT_TOSERVER) {
SCLogDebug("mpm inspection");
if (det_ctx->sgh->flags & SIG_GROUP_HEAD_MPM_DNSQUERY) {
FLOWLOCK_RDLOCK(p->flow);
void *alstate = FlowGetAppState(p->flow);
if (alstate == NULL) {
SCLogDebug("no alstate");
FLOWLOCK_UNLOCK(p->flow);
return;
}
@ -1174,7 +1157,6 @@ static inline void DetectMpmPrefilter(DetectEngineCtx *de_ctx,
DetectDnsQueryInspectMpm(det_ctx, p->flow, alstate, flags, tx, idx);
PACKET_PROFILING_DETECT_END(p, PROF_DETECT_MPM_DNSQUERY);
}
FLOWLOCK_UNLOCK(p->flow);
}
}
}
@ -1353,7 +1335,6 @@ int SigMatchSignatures(ThreadVars *th_v, DetectEngineCtx *de_ctx, DetectEngineTh
SCLogDebug("STREAM_EOF set");
}
FLOWLOCK_WRLOCK(pflow);
{
/* store tenant_id in the flow so that we can use it
* for creating pseudo packets */
@ -1436,7 +1417,6 @@ int SigMatchSignatures(ThreadVars *th_v, DetectEngineCtx *de_ctx, DetectEngineTh
pflow->alparser,
flow_flags);
}
FLOWLOCK_UNLOCK(pflow);
if (((p->flowflags & FLOW_PKT_TOSERVER) && !(p->flowflags & FLOW_PKT_TOSERVER_IPONLY_SET)) ||
((p->flowflags & FLOW_PKT_TOCLIENT) && !(p->flowflags & FLOW_PKT_TOCLIENT_IPONLY_SET)))
@ -1447,8 +1427,7 @@ int SigMatchSignatures(ThreadVars *th_v, DetectEngineCtx *de_ctx, DetectEngineTh
IPOnlyMatchPacket(th_v, de_ctx, det_ctx, &de_ctx->io_ctx, &det_ctx->io_ctx, p);
PACKET_PROFILING_DETECT_END(p, PROF_DETECT_IPONLY);
/* save in the flow that we scanned this direction... locking is
* done in the FlowSetIPOnlyFlag function. */
/* save in the flow that we scanned this direction... */
FlowSetIPOnlyFlag(pflow, p->flowflags & FLOW_PKT_TOSERVER ? 1 : 0);
} else if (((p->flowflags & FLOW_PKT_TOSERVER) &&
@ -1474,9 +1453,7 @@ int SigMatchSignatures(ThreadVars *th_v, DetectEngineCtx *de_ctx, DetectEngineTh
#ifdef DEBUG
if (pflow) {
SCMutexLock(&pflow->m);
DebugInspectIds(p, pflow, smsg);
SCMutexUnlock(&pflow->m);
}
#endif
} else { /* p->flags & PKT_HAS_FLOW */
@ -1627,9 +1604,7 @@ int SigMatchSignatures(ThreadVars *th_v, DetectEngineCtx *de_ctx, DetectEngineTh
* and if so, if we actually have any in the flow. If not, the sig
* can't match and we skip it. */
if ((p->flags & PKT_HAS_FLOW) && (sflags & SIG_FLAG_REQUIRE_FLOWVAR)) {
FLOWLOCK_RDLOCK(pflow);
int m = pflow->flowvar ? 1 : 0;
FLOWLOCK_UNLOCK(pflow);
/* no flowvars? skip this sig */
if (m == 0) {
@ -1868,7 +1843,6 @@ end:
* up again for the next packet. Also return any stream chunk we processed
* to the pool. */
if (p->flags & PKT_HAS_FLOW) {
FLOWLOCK_WRLOCK(pflow);
if (debuglog_enabled) {
if (p->alerts.cnt > 0) {
AlertDebugLogModeSyncFlowbitsNamesToPacketStruct(p, de_ctx);
@ -1946,8 +1920,6 @@ end:
/* if we had no alerts that involved the smsgs,
* we can get rid of them now. */
StreamMsgReturnListToPool(smsg);
FLOWLOCK_UNLOCK(pflow);
}
PACKET_PROFILING_DETECT_END(p, PROF_DETECT_CLEANUP);
@ -2079,12 +2051,15 @@ TmEcode Detect(ThreadVars *tv, Packet *p, void *data, PacketQueue *pq, PacketQue
}
if (p->flow) {
det_ctx->flow_locked = 1;
FLOWLOCK_WRLOCK(p->flow);
DetectFlow(tv, de_ctx, det_ctx, p);
FLOWLOCK_UNLOCK(p->flow);
det_ctx->flow_locked = 0;
} else {
DetectNoFlow(tv, de_ctx, det_ctx, p);
}
return TM_ECODE_OK;
error:
return TM_ECODE_FAILED;
}

@ -125,28 +125,24 @@ void FlowBitToggle(Flow *f, uint16_t idx)
int FlowBitIsset(Flow *f, uint16_t idx)
{
int r = 0;
FLOWLOCK_RDLOCK(f);
FlowBit *fb = FlowBitGet(f, idx);
if (fb != NULL) {
r = 1;
}
FLOWLOCK_UNLOCK(f);
return r;
}
int FlowBitIsnotset(Flow *f, uint16_t idx)
{
int r = 0;
FLOWLOCK_RDLOCK(f);
FlowBit *fb = FlowBitGet(f, idx);
if (fb == NULL) {
r = 1;
}
FLOWLOCK_UNLOCK(f);
return r;
}

@ -147,26 +147,12 @@ int FlowUpdateSpareFlows(void)
return 1;
}
/** \brief Set the IPOnly scanned flag for 'direction'. This function
* handles the locking too.
* \param f Flow to set the flag in
* \param direction direction to set the flag in
*/
void FlowSetIPOnlyFlag(Flow *f, char direction)
{
FLOWLOCK_WRLOCK(f);
direction ? (f->flags |= FLOW_TOSERVER_IPONLY_SET) :
(f->flags |= FLOW_TOCLIENT_IPONLY_SET);
FLOWLOCK_UNLOCK(f);
return;
}
/** \brief Set the IPOnly scanned flag for 'direction'.
*
* \param f Flow to set the flag in
* \param direction direction to set the flag in
*/
void FlowSetIPOnlyFlagNoLock(Flow *f, char direction)
void FlowSetIPOnlyFlag(Flow *f, int direction)
{
direction ? (f->flags |= FLOW_TOSERVER_IPONLY_SET) :
(f->flags |= FLOW_TOCLIENT_IPONLY_SET);

@ -441,8 +441,7 @@ void FlowHandlePacket (ThreadVars *, DecodeThreadVars *, Packet *);
void FlowInitConfig (char);
void FlowPrintQueueInfo (void);
void FlowShutdown(void);
void FlowSetIPOnlyFlag(Flow *, char);
void FlowSetIPOnlyFlagNoLock(Flow *, char);
void FlowSetIPOnlyFlag(Flow *, int);
void FlowRegisterTests (void);
int FlowSetProtoTimeout(uint8_t ,uint32_t ,uint32_t ,uint32_t);

Loading…
Cancel
Save