From: Ken Steele Date: Thu, 6 Nov 2014 19:57:53 +0000 (-0500) Subject: Fix bug in MPM rule array handling X-Git-Tag: suricata-2.1beta3~36 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1c03eb56d0a9af1ca8015638d96b0f676c010ff3;p=thirdparty%2Fsuricata.git Fix bug in MPM rule array handling In PmqMerge() use MpmAddSids() instead of blindly copying the src rule list onto the end of the dst rule list, since there might not be enough room in the dst list. MpmAddSids() will resize the dst array if needed. Also add code to MpmAddSids() MpmAddPid() to better handle the case that realloc fails to get more space. It first tries 2x the needed space, but if that fails, it tries for just 1x. If that fails resize returns 0. For MpmAddPid(), if resize fails, the new pid is lost. For MpmAddSids(), as many SIDs as will fit are added, but some will be lost. --- diff --git a/src/util-mpm.c b/src/util-mpm.c index 5a90e36139..9a3da161d0 100644 --- a/src/util-mpm.c +++ b/src/util-mpm.c @@ -473,7 +473,7 @@ int PmqSetup(PatternMatcherQueue *pmq, uint32_t patmaxid) * \param new_size number of Signature IDs needing to be stored. * */ -void +int MpmAddSidsResize(PatternMatcherQueue *pmq, uint32_t new_size) { /* Need to make the array bigger. Double the size needed to @@ -484,12 +484,21 @@ MpmAddSidsResize(PatternMatcherQueue *pmq, uint32_t new_size) uint32_t *new_array = (uint32_t*)SCRealloc(pmq->rule_id_array, new_size * sizeof(uint32_t)); if (unlikely(new_array == NULL)) { - SCLogError(SC_ERR_MEM_ALLOC, "Failed to realloc PatternMatchQueue" - " rule ID array. Some signature ID matches lost"); - return; + /* Try again just big enough. */ + new_size = new_size / 2; + new_array = (SigIntId*)SCRealloc(pmq->rule_id_array, + new_size * sizeof(SigIntId)); + if (unlikely(new_array == NULL)) { + + SCLogError(SC_ERR_MEM_ALLOC, "Failed to realloc PatternMatchQueue" + " rule ID array. Some signature ID matches lost"); + return 0; + } } pmq->rule_id_array = new_array; pmq->rule_id_array_size = new_size; + + return new_size; } /** \brief Increase the size of the Pattern rule ID array. @@ -497,8 +506,9 @@ MpmAddSidsResize(PatternMatcherQueue *pmq, uint32_t new_size) * \param pmq storage for match results * \param new_size number of Signature IDs needing to be stored. * + * \return 0 on failure. */ -void +int MpmAddPidResize(PatternMatcherQueue *pmq, uint32_t new_size) { /* Need to make the array bigger. Double the size needed to @@ -509,12 +519,20 @@ MpmAddPidResize(PatternMatcherQueue *pmq, uint32_t new_size) uint32_t *new_array = (uint32_t*)SCRealloc(pmq->pattern_id_array, new_size * sizeof(uint32_t)); if (unlikely(new_array == NULL)) { - SCLogError(SC_ERR_MEM_ALLOC, "Failed to realloc PatternMatchQueue" - " pattern ID array. Some new Pattern ID matches were lost."); - return; + // Failed to allocate 2x, so try 1x. + new_size = new_size / 2; + new_array = (uint32_t*)SCRealloc(pmq->pattern_id_array, + new_size * sizeof(uint32_t)); + if (unlikely(new_array == NULL)) { + SCLogError(SC_ERR_MEM_ALLOC, "Failed to realloc PatternMatchQueue" + " pattern ID array. Some new Pattern ID matches were lost."); + return 0; + } } pmq->pattern_id_array = new_array; pmq->pattern_id_array_size = new_size; + + return new_size; } /** \brief Verify and store a match @@ -573,11 +591,7 @@ void PmqMerge(PatternMatcherQueue *src, PatternMatcherQueue *dst) /** \todo now set merged flag? */ - if (src->rule_id_array && dst->rule_id_array) { - for (u = 0; u < src->rule_id_array_cnt; u++) { - dst->rule_id_array[dst->rule_id_array_cnt++] = src->rule_id_array[u]; - } - } + MpmAddSids(dst, src->rule_id_array, src->rule_id_array_cnt); } /** \brief Reset a Pmq for reusage. Meant to be called after a single search. diff --git a/src/util-mpm.h b/src/util-mpm.h index d1ebeef8d9..e88e717194 100644 --- a/src/util-mpm.h +++ b/src/util-mpm.h @@ -267,7 +267,7 @@ int MpmAddPatternCI(struct MpmCtx_ *mpm_ctx, uint8_t *pat, uint16_t patlen, uint32_t pid, uint32_t sid, uint8_t flags); /* Resize Signature ID array. Only called from MpmAddSids(). */ -void MpmAddSidsResize(PatternMatcherQueue *pmq, uint32_t new_size); +int MpmAddSidsResize(PatternMatcherQueue *pmq, uint32_t new_size); /** \brief Add array of Signature IDs to rule ID array. * @@ -284,7 +284,11 @@ MpmAddSids(PatternMatcherQueue *pmq, uint32_t *sids, uint32_t sids_size) { uint32_t new_size = pmq->rule_id_array_cnt + sids_size; if (new_size > pmq->rule_id_array_size) { - MpmAddSidsResize(pmq, new_size); + if (MpmAddSidsResize(pmq, new_size) == 0) { + // Failed to allocate larger memory for all the SIDS, but + // keep as many as we can. + sids_size = pmq->rule_id_array_size - pmq->rule_id_array_cnt; + } } SCLogDebug("Adding %u sids", sids_size); // Add SIDs for this pattern to the end of the array @@ -294,14 +298,16 @@ MpmAddSids(PatternMatcherQueue *pmq, uint32_t *sids, uint32_t sids_size) } /* Resize Pattern ID array. Only called from MpmAddPid(). */ -void MpmAddPidResize(PatternMatcherQueue *pmq, uint32_t new_size); +int MpmAddPidResize(PatternMatcherQueue *pmq, uint32_t new_size); static inline void MpmAddPid(PatternMatcherQueue *pmq, uint32_t patid) { uint32_t new_size = pmq->pattern_id_array_cnt + 1; - if (new_size > pmq->pattern_id_array_size) - MpmAddPidResize(pmq, new_size); + if (new_size > pmq->pattern_id_array_size) { + if (MpmAddPidResize(pmq, new_size) == 0) + return; + } pmq->pattern_id_array[pmq->pattern_id_array_cnt] = patid; pmq->pattern_id_array_cnt = new_size; SCLogDebug("pattern_id_array_cnt %u", pmq->pattern_id_array_cnt);