]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
Fix bug in MPM rule array handling
authorKen Steele <ken@tilera.com>
Thu, 6 Nov 2014 19:57:53 +0000 (14:57 -0500)
committerVictor Julien <victor@inliniac.net>
Thu, 15 Jan 2015 10:52:03 +0000 (11:52 +0100)
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.

src/util-mpm.c
src/util-mpm.h

index 5a90e36139b9d78481a32330368d989e99a333f4..9a3da161d03101c3149f27c1b5d7bb201355f213 100644 (file)
@@ -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.
index d1ebeef8d9183f6bbda52cc1a221f5f6f2f21d30..e88e717194a58dbbb50287ee65b0c056ff5fbb40 100644 (file)
@@ -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);