View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0000524LDMud 3.3Implementationpublic2008-01-21 04:092018-01-29 22:57
Reporterzesstra Assigned Tozesstra  
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.3 
Fixed in Version3.3.717 
Summary0000524: Crash in PCRE-RegExp efuns due to excessive recursion
DescriptionThe function match() in the PCRE package is highly recursive. Depending on the string and the regular expression it can recurse thousands of times.

Something like
regmatch("aaa bbb ccc"*1000,"a{1,2}(\n|.)+cc",RE_PCRE|RE_MULTILINE|RE_MATCH_SUBS,0)
causes the driver to crash with a segmentation fault (EXC_BAD_ACCESS).
I had a quick look at the core dump, there were some 18k frames on the stack. Basically, match() recursed until it reached the stack size limit (8 MB at that time) of the OS and the process was killed.
Raising the limit to 16MB solved the problem for this regmatch call, but it doesn't solve the general problem. Using the maximum stack size of 64MB on my machine required just a longer string, until the stack was filled with 144k frames ;-)
There should definitely be some safeguards (other than eval costs) for preventing to crash the driver in this way, you never know when someone puts a whole file in a RegExp-Efun or even tries to deliberately crash the driver.

I briefly thought about solutions:
1. configure the PCRE code with NO_RECURSE not to use recursions but a
   private stack on the heap, but it is noticeably slower then and we would
   have to set 2 function pointers for using the driver memory allocation
   handler instead of standard malloc. Additionally, allocating an indefinite
   amount of memory on the heap for match() only reduces the problem.
2. limit the size of the strings to match. This is probably a sane thing, but
   may be difficult as the amount of recursions also depend on the pattern
   and one would have to set the limit suitable to the given stack size
   limit.
3. change the PCRE code to keep track of the stack size and abort the
   process with a PCRE_ERROR_xxx if it approaches the stack size limit. The
   calling RegExp efuns could then throw an error. Don't know if there is
   a library/system call for finding the current stack size...
   On could calculate it from the current stack pointer and the stack base
   adress, but... *sigh*
4. check for updates of PCRE with improved recursion behaviour.
5. ... ?
Steps To Reproduceregmatch("aaa bbb ccc"*1000,"a{1,2}(\n|.)+cc",RE_PCRE|RE_MULTILINE|RE_MATCH_SUBS,0)
TagsNo tags attached.

Activities

zesstra

2008-01-21 05:28

administrator   ~0000580

Ok. There already is some way to limit the recursions in PCRE. ;-) And the driver even uses it, but...
The specific lines are 679 and 742 in mregex.c:
pHints->match_limit = max_eval_cost - eval_cost - 1;

This is far too much for the match limit, I think. We have 1.5MTicks as Eval cost limit in MorgenGrauen right now, but even 100k would be enough for the standard stack size limit of 8M.

I would suggest to limit 'match_limit' to a maximum defined either at compile time or some run-time defined value which takes the current stack size limit into account in some way. (Unfortunately, PCRE limits the number of recursions, but not the used stack size, so we have to assume some values for 'used size per recursion' here.)
(Of course, if there are less eval ticks available than this maximum amount of recursions, we should use the available ticks.)

Additionally there are cases (if pHint is not present), where there is no match_limit set. We should either change them or define some sane maximum in PCRE's config.h. In fact, the regmatch() call here doesn't have any match_limit set and defaults then to 10000000 from pcre/config.h.

Gnomi

2008-01-21 05:53

manager   ~0000581

At home I have 1008 bytes per recursion, on the UNItopia machine it is 592 bytes per recursion (both are Debian packages of PCRE 7.4, but different Debian releases).

zesstra

2008-01-21 06:10

administrator   ~0000582

Mhmm. Interesting difference.
I just found a curious thing: as I said, in my regmatch() case there was no pHints present and therefore no specific match_limit. pHint is generated by pcre_study() in pcre/study.c but despite various RegExp I could not get a single case, where pcre_study() actually returned a non-zero pHints. I could be wrong, but at least here pHints->match_limit seems to be never set.
If we want to use that mechanism we have to set either the global maximum or make sure, there is an initialized pHints member in the RegExp struct.

zesstra

2008-01-21 07:15

administrator   ~0000583

BTW: My tests here at home were done with the built-in PCRE package, configure does not seem to use/find my system pcrelib. Anyway, it eases debugging. ;-)
I had a look at the newer PCRE versions. Most recent version is 7.5, the driver ships with 4.5. According to the changelog there were some improvements in saving stack space. Additionally, they have now a MATCH_LIMIT_RECURSION which limits the recursion depth of match() while MATCH_LIMIT limits the overall number of calls to match().
An update of the built-in PCRE would be nice, unfortunately, the changes to the PCRE package seem to be quite extensive.
I think, we should make sure each call to pcre_exec() has a sane match_limit or match_limit_recursion set instead of setting the global compile-time maximum, because that way only works while the built-in copy of PCRE is used.

2008-01-25 06:40

 

pcre_limit.diff (6,694 bytes)    
Index: trunk/src/mregex.c
===================================================================
--- trunk/src/mregex.c	(Revision 2364)
+++ trunk/src/mregex.c	(Arbeitskopie)
@@ -408,6 +408,21 @@
             errorf("pcre: %s\n", pErrmsg);
         return 0;
     }
+    /* We have to ensure to have an initialized pHints structure for
+       setting the recursion limit later on */
+    if (pHints == NULL) 
+    {
+        pcre_malloc_err = "allocating memory for pHints section in regexp";
+        pHints = (pcre_extra *)pcre_xalloc(sizeof(pcre_extra));
+        if (pHints == NULL) 
+        {
+            if (from_ed)
+                add_message("pcre: Could not allocate memory for pHints\n");
+            else
+                errorf("pcre: Could not allocate memory for pHints\n");
+            return 0;
+        }
+    }
 
     {
        int rc;
@@ -672,18 +687,28 @@
         if (prog->opt & RE_NOTEMPTY) pcre_opt |= PCRE_NOTEMPTY;
 
         pHints = prog->pHints;
-        if (pHints && max_eval_cost)
-        {
-            pHints->flags |= PCRE_EXTRA_MATCH_LIMIT;
-            if (max_eval_cost > eval_cost + 1)
-                pHints->match_limit = max_eval_cost - eval_cost - 1;
-            else
-                pHints->match_limit = 1;
-        }
-        else if (pHints)
-        {
-            pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT;
-        }
+        /* If PCRE_RECURSION_LIMIT is defined we set a limit for match. If
+         * PCRE_EXTRA_MATCH_LIMIT_RECURSION is defined, we have a new libpcre,
+         * which supports limiting the recursions. Otherwise we have to limit
+         * the no of calls to match().
+         * TODO: Instead of the conditional compilation we should update the
+         * TODO::built-in pcre package.
+         */
+#ifdef PCRE_RECURSION_LIMIT
+#ifdef PCRE_EXTRA_MATCH_LIMIT_RECURSION
+        pHints->flags |= PCRE_EXTRA_MATCH_LIMIT_RECURSION;
+        pHints->match_limit_recursion = PCRE_RECURSION_LIMIT;
+#else
+        pHints->flags |= PCRE_EXTRA_MATCH_LIMIT;
+        pHints->match_limit = PCRE_RECURSION_LIMIT;
+#endif /* PCRE_EXTRA_MATCH_LIMIT_RECURSION */
+#else  /* PCRE_RECURSION_LIMIT */
+#ifdef PCRE_EXTRA_MATCH_LIMIT_RECURSION
+        pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT_RECURSION
+#else
+        pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT;
+#endif  /* PCRE_EXTRA_MATCH_LIMIT_RECURSION */
+#endif  /* PCRE_RECURSION_LIMIT */
 
         rc = pcre_exec( prog->pProg, pHints
                       , get_txt(string), mstrsize(string), start, pcre_opt
@@ -735,18 +760,28 @@
         if (prog->opt & RE_NOTEMPTY) pcre_opt |= PCRE_NOTEMPTY;
 
         pHints = prog->pHints;
-        if (pHints && max_eval_cost)
-        {
-            pHints->flags |= PCRE_EXTRA_MATCH_LIMIT;
-            if (max_eval_cost > eval_cost + 1)
-                pHints->match_limit = max_eval_cost - eval_cost - 1;
-            else
-                pHints->match_limit = 1;
-        }
-        else if (pHints)
-        {
-            pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT;
-        }
+        /* If PCRE_RECURSION_LIMIT is defined we set a limit for match. If
+         * PCRE_EXTRA_MATCH_LIMIT_RECURSION is defined, we have a new libpcre,
+         * which supports limiting the recursions. Otherwise we have to limit
+         * the no of calls to match().
+         * TODO: Instead of the conditional compilation we should update the
+         * TODO::built-in pcre package.
+         */
+#ifdef PCRE_RECURSION_LIMIT
+#ifdef PCRE_EXTRA_MATCH_LIMIT_RECURSION
+        pHints->flags |= PCRE_EXTRA_MATCH_LIMIT_RECURSION;
+        pHints->match_limit_recursion = PCRE_RECURSION_LIMIT;
+#else
+        pHints->flags |= PCRE_EXTRA_MATCH_LIMIT;
+        pHints->match_limit = PCRE_RECURSION_LIMIT;
+#endif /* PCRE_EXTRA_MATCH_LIMIT_RECURSION */
+#else  /* PCRE_RECURSION_LIMIT */
+#ifdef PCRE_EXTRA_MATCH_LIMIT_RECURSION
+        pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT_RECURSION
+#else
+        pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT;
+#endif  /* PCRE_EXTRA_MATCH_LIMIT_RECURSION */
+#endif  /* PCRE_RECURSION_LIMIT */
 
         rc = pcre_exec( prog->pProg, pHints
                       , start, strlen(start), string - start, pcre_opt
Index: trunk/src/autoconf/configure.in
===================================================================
--- trunk/src/autoconf/configure.in	(Revision 2364)
+++ trunk/src/autoconf/configure.in	(Arbeitskopie)
@@ -217,6 +217,7 @@
 AC_MY_ARG_WITH(min-small-malloced,0,,)
 AC_MY_ARG_WITH(max-malloced,0x4000000,,)
 AC_MY_ARG_WITH(total-trace-length,0x1000,,)
+AC_MY_ARG_WITH(pcre-recursion-limit,7000,,[maximum number of recursions in PCRE package])
 AC_MY_ARG_WITH(wizlist-file,WIZLIST,,[name of the wizlist file])
 
 AC_ARG_WITH(setting,[  --with-setting=SETTING  include a predefined setting],[
@@ -444,6 +445,7 @@
 AC_INT_VAL_FROM_WITH(min_small_malloced)
 AC_INT_VAL_FROM_WITH(max_malloced)
 AC_INT_VAL_FROM_WITH(total_trace_length)
+AC_INT_VAL_FROM_WITH(pcre_recursion_limit)
 
 if test "x$cdef_access_control" = "x#undef"; then
   cdef_access_log="#undef"
@@ -2739,6 +2741,7 @@
 AC_SUBST(val_max_malloced)
 AC_SUBST(val_total_trace_length)
 AC_SUBST(val_wizlist_file)
+AC_SUBST(val_pcre_recursion_limit)
 
 dnl finally: some remaining stuff
 dnl
Index: trunk/src/config.h.in
===================================================================
--- trunk/src/config.h.in	(Revision 2364)
+++ trunk/src/config.h.in	(Arbeitskopie)
@@ -434,6 +434,16 @@
 #define ALLOWED_ED_CMDS           @val_allowed_ed_cmds@
 /* TODO: ALLOWED_ED_CMDS: make this a runtime option */
 
+/* Limit the amount of recursion in the PCRE code. Setting it to low will
+ * prevent certain regexps to be executed properly, setting it too high can
+ * cause that regexps to crash the driver. Set it according to the
+ * available maximum stack size for the driver process. (Rule of thumb:
+ * The memory used for a recursion on the stack seems to be within 466 and
+ * 1008 bytes. If you have 8M of stack size, choose 7000 - 8000.)
+ * Defaults to 7000
+ */                                                                                       
+#define PCRE_RECURSION_LIMIT    @val_pcre_recursion_limit@                                
+			
 
 /* --- Compiler --- */
 
Index: trunk/src/pkg-pcre.h
===================================================================
--- trunk/src/pkg-pcre.h	(Revision 2364)
+++ trunk/src/pkg-pcre.h	(Arbeitskopie)
@@ -22,6 +22,10 @@
 
 /* Error code to be returned if too many backtracks are detected.
  */
+#ifdef PCRE_ERROR_RECURSIONLIMIT
+#define RE_ERROR_BACKTRACK PCRE_ERROR_RECURSIONLIMIT
+#else
 #define RE_ERROR_BACKTRACK PCRE_ERROR_MATCHLIMIT
+#endif
 
 #endif /* PKG_PCRE_H_ */
pcre_limit.diff (6,694 bytes)   

zesstra

2008-01-25 06:41

administrator   ~0000584

I assembled a patch for limiting the recursions in match(), which introduces
the following changes:
* ensure a initialized pHints section after call to pcre_study() in
  rx_compile()
* set a limit for match in rc_exec() if PCRE_RECURSION_LIMIT is defined.
  If we use a newer libpcre with support for PCRE_EXTRA_MATCH_LIMIT_RECURSION
  we use that, otherwise PCRE_EXTRA_MATCH_LIMIT
* set RE_ERROR_BACKTRACK to PCRE_ERROR_RECURSIONLIMIT if defined or
  PCRE_ERROR_MATCHLIMIT otherwise
* I did not make PCRE_RECURSION_LIMIT changeable at run-time. Increasing it
  would anyway only make sense if you increase the stack size limit of the
  driver process.
* I removed the dependence of the recursion limit on the remaining
  eval_costs, sensible recursion limits on most systems would probably
  be in the range of 10k - 20k recursions.
* added PCRE_RECURSION_LIMIT to configure.in with a default of 7000.
* added PCRE_RECURSION_LIMIT to config.in with a describing comment

The default limit is a conservative guess, I think. Gnomi and me had for
several versions and compilations of the libpcre frame sizes between 466
and 1008 bytes and I assume a default 8M for the stack size limit.

Several things could still be improved, but I don't know if that is needed,
they are more or less complicated to do:
* make PCRE_RECURSION_LIMIT run-time configurable, a pre-defined LPC define
  or include it in query_limits()
* update the built-in pcre package (would be a good idea, but I don't have
  time for that at the moment)
* don't limit the number of recursions, but the amount of stack space used.
  That would require a patch for libpcre itself and is probably not
  feasible
* calculate the recursion limit dynamically in rx_exec(), depending on the
  stacke size limit (getrlimit()) and the already consumed stack space
  (not straight-forward, there is no libc function for that, one would
   have to calculate it from the top and bottom of the stack)
* PCRE_RECURSION_LIMIT has a small chance of provoking a name clash with
  the libpcre defines in the future. It may be reasonable to rename it to
  something like LD_PCRE_RECURSION_LIMIT

Comments, Complaints, Requests welcome. ;-)

Gnomi

2008-02-20 08:21

manager   ~0000602

In the patch when allocating a new pcre_extra structure pHints->flags should be set to zero, otherwise pcre may crash later.

zesstra

2008-02-20 09:27

administrator   ~0000603

Hmpf. Right. I will update the patch soon, thank you.
I was probably thinking of calloc from ptmalloc which zeroes out allocated memory. or simply forgot it. *g*
But, I still don't like the patch very much, because it is completely static and doesn't take into account the current stack usage. Maybe it would really be better to check the stack size first (get_stack_direction() in xalloc.c in fact keeps track of (more or less) the beginning of the stack, don't know if it is worth to add some get_stack_size() function there?)

2008-04-01 05:18

 

pcre_limit_v2.diff (6,722 bytes)    
Index: trunk/src/mregex.c
===================================================================
--- trunk/src/mregex.c	(Revision 2364)
+++ trunk/src/mregex.c	(Arbeitskopie)
@@ -408,6 +408,22 @@
             errorf("pcre: %s\n", pErrmsg);
         return 0;
     }
+    /* We have to ensure to have an initialized pHints structure for
+       setting the recursion limit later on */
+    if (pHints == NULL) 
+    {
+        pcre_malloc_err = "allocating memory for pHints section in regexp";
+        pHints = (pcre_extra *)pcre_xalloc(sizeof(pcre_extra));
+        if (pHints == NULL) 
+        {
+            if (from_ed)
+                add_message("pcre: Could not allocate memory for pHints\n");
+            else
+                errorf("pcre: Could not allocate memory for pHints\n");
+            return 0;
+        }
+        pHints->flags = 0;
+    }
 
     {
        int rc;
@@ -672,18 +687,28 @@
         if (prog->opt & RE_NOTEMPTY) pcre_opt |= PCRE_NOTEMPTY;
 
         pHints = prog->pHints;
-        if (pHints && max_eval_cost)
-        {
-            pHints->flags |= PCRE_EXTRA_MATCH_LIMIT;
-            if (max_eval_cost > eval_cost + 1)
-                pHints->match_limit = max_eval_cost - eval_cost - 1;
-            else
-                pHints->match_limit = 1;
-        }
-        else if (pHints)
-        {
-            pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT;
-        }
+        /* If PCRE_RECURSION_LIMIT is defined we set a limit for match. If
+         * PCRE_EXTRA_MATCH_LIMIT_RECURSION is defined, we have a new libpcre,
+         * which supports limiting the recursions. Otherwise we have to limit
+         * the no of calls to match().
+         * TODO: Instead of the conditional compilation we should update the
+         * TODO::built-in pcre package.
+         */
+#ifdef PCRE_RECURSION_LIMIT
+#ifdef PCRE_EXTRA_MATCH_LIMIT_RECURSION
+        pHints->flags |= PCRE_EXTRA_MATCH_LIMIT_RECURSION;
+        pHints->match_limit_recursion = PCRE_RECURSION_LIMIT;
+#else
+        pHints->flags |= PCRE_EXTRA_MATCH_LIMIT;
+        pHints->match_limit = PCRE_RECURSION_LIMIT;
+#endif /* PCRE_EXTRA_MATCH_LIMIT_RECURSION */
+#else  /* PCRE_RECURSION_LIMIT */
+#ifdef PCRE_EXTRA_MATCH_LIMIT_RECURSION
+        pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT_RECURSION
+#else
+        pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT;
+#endif  /* PCRE_EXTRA_MATCH_LIMIT_RECURSION */
+#endif  /* PCRE_RECURSION_LIMIT */
 
         rc = pcre_exec( prog->pProg, pHints
                       , get_txt(string), mstrsize(string), start, pcre_opt
@@ -735,18 +760,28 @@
         if (prog->opt & RE_NOTEMPTY) pcre_opt |= PCRE_NOTEMPTY;
 
         pHints = prog->pHints;
-        if (pHints && max_eval_cost)
-        {
-            pHints->flags |= PCRE_EXTRA_MATCH_LIMIT;
-            if (max_eval_cost > eval_cost + 1)
-                pHints->match_limit = max_eval_cost - eval_cost - 1;
-            else
-                pHints->match_limit = 1;
-        }
-        else if (pHints)
-        {
-            pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT;
-        }
+        /* If PCRE_RECURSION_LIMIT is defined we set a limit for match. If
+         * PCRE_EXTRA_MATCH_LIMIT_RECURSION is defined, we have a new libpcre,
+         * which supports limiting the recursions. Otherwise we have to limit
+         * the no of calls to match().
+         * TODO: Instead of the conditional compilation we should update the
+         * TODO::built-in pcre package.
+         */
+#ifdef PCRE_RECURSION_LIMIT
+#ifdef PCRE_EXTRA_MATCH_LIMIT_RECURSION
+        pHints->flags |= PCRE_EXTRA_MATCH_LIMIT_RECURSION;
+        pHints->match_limit_recursion = PCRE_RECURSION_LIMIT;
+#else
+        pHints->flags |= PCRE_EXTRA_MATCH_LIMIT;
+        pHints->match_limit = PCRE_RECURSION_LIMIT;
+#endif /* PCRE_EXTRA_MATCH_LIMIT_RECURSION */
+#else  /* PCRE_RECURSION_LIMIT */
+#ifdef PCRE_EXTRA_MATCH_LIMIT_RECURSION
+        pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT_RECURSION
+#else
+        pHints->flags &= ~PCRE_EXTRA_MATCH_LIMIT;
+#endif  /* PCRE_EXTRA_MATCH_LIMIT_RECURSION */
+#endif  /* PCRE_RECURSION_LIMIT */
 
         rc = pcre_exec( prog->pProg, pHints
                       , start, strlen(start), string - start, pcre_opt
Index: trunk/src/autoconf/configure.in
===================================================================
--- trunk/src/autoconf/configure.in	(Revision 2364)
+++ trunk/src/autoconf/configure.in	(Arbeitskopie)
@@ -217,6 +217,7 @@
 AC_MY_ARG_WITH(min-small-malloced,0,,)
 AC_MY_ARG_WITH(max-malloced,0x4000000,,)
 AC_MY_ARG_WITH(total-trace-length,0x1000,,)
+AC_MY_ARG_WITH(pcre-recursion-limit,7000,,[maximum number of recursions in PCRE package])
 AC_MY_ARG_WITH(wizlist-file,WIZLIST,,[name of the wizlist file])
 
 AC_ARG_WITH(setting,[  --with-setting=SETTING  include a predefined setting],[
@@ -444,6 +445,7 @@
 AC_INT_VAL_FROM_WITH(min_small_malloced)
 AC_INT_VAL_FROM_WITH(max_malloced)
 AC_INT_VAL_FROM_WITH(total_trace_length)
+AC_INT_VAL_FROM_WITH(pcre_recursion_limit)
 
 if test "x$cdef_access_control" = "x#undef"; then
   cdef_access_log="#undef"
@@ -2739,6 +2741,7 @@
 AC_SUBST(val_max_malloced)
 AC_SUBST(val_total_trace_length)
 AC_SUBST(val_wizlist_file)
+AC_SUBST(val_pcre_recursion_limit)
 
 dnl finally: some remaining stuff
 dnl
Index: trunk/src/config.h.in
===================================================================
--- trunk/src/config.h.in	(Revision 2364)
+++ trunk/src/config.h.in	(Arbeitskopie)
@@ -434,6 +434,16 @@
 #define ALLOWED_ED_CMDS           @val_allowed_ed_cmds@
 /* TODO: ALLOWED_ED_CMDS: make this a runtime option */
 
+/* Limit the amount of recursion in the PCRE code. Setting it to low will
+ * prevent certain regexps to be executed properly, setting it too high can
+ * cause that regexps to crash the driver. Set it according to the
+ * available maximum stack size for the driver process. (Rule of thumb:
+ * The memory used for a recursion on the stack seems to be within 466 and
+ * 1008 bytes. If you have 8M of stack size, choose 7000 - 8000.)
+ * Defaults to 7000
+ */                                                                                       
+#define PCRE_RECURSION_LIMIT    @val_pcre_recursion_limit@                                
+			
 
 /* --- Compiler --- */
 
Index: trunk/src/pkg-pcre.h
===================================================================
--- trunk/src/pkg-pcre.h	(Revision 2364)
+++ trunk/src/pkg-pcre.h	(Arbeitskopie)
@@ -22,6 +22,10 @@
 
 /* Error code to be returned if too many backtracks are detected.
  */
+#ifdef PCRE_ERROR_RECURSIONLIMIT
+#define RE_ERROR_BACKTRACK PCRE_ERROR_RECURSIONLIMIT
+#else
 #define RE_ERROR_BACKTRACK PCRE_ERROR_MATCHLIMIT
+#endif
 
 #endif /* PKG_PCRE_H_ */
pcre_limit_v2.diff (6,722 bytes)   

zesstra

2008-04-01 06:01

administrator   ~0000607

Of course I forget this issue again for some time. ;-) Remembered it this morning and added the initialisation of pHints->flags. New patch is pcre_limit_v2.diff.

zesstra

2008-05-06 15:49

administrator   ~0000614

As I wrote, my current patch has the disadvantage, that it is static. Therefore the driver can still crash if the recursion is already quite deep before executing the RegExp and a low limit may prevent legimate RegExps from being executed correctly.
I could add functions in xalloc.c (because there the stack direction is already known) to find out about the current stack usage und the remaining space. Then the Regexp package can limit to recursions dynamically (but with the assumption that one recursion takes about 1k of stack space).
Is there any interest for that approach?

Gnomi

2008-07-01 04:50

manager   ~0000639

I can live with the static limit. The dynamic limit would be an approximation as well (because we can't determine the needed stack space per recursion), as LPC has a recursion limit (80 in UNItopia), which should never exceed half of the stack space, I can give PCRE the other half of the stack and that should suffice.

zesstra

2008-07-07 05:39

administrator   ~0000673

True enough. I will go for the static limit, if there are too many problems, we can still change it. Maybe I will expand the comment about the limit a little bit to clarify that users have to choose according to the available stack size and that they should choose suitable for worst conditions (max. LPC recursion, when executing a PCRE).
BTW: Testcase in SVN, r2374.

zesstra

2008-07-07 17:37

administrator   ~0000674

This should be fixed by r2375. In case of problems with the static limit (or something else), please re-open.

Issue History

Date Modified Username Field Change
2008-01-21 04:09 zesstra New Issue
2008-01-21 05:28 zesstra Note Added: 0000580
2008-01-21 05:53 Gnomi Note Added: 0000581
2008-01-21 06:10 zesstra Note Added: 0000582
2008-01-21 07:15 zesstra Note Added: 0000583
2008-01-25 06:40 zesstra File Added: pcre_limit.diff
2008-01-25 06:41 zesstra Note Added: 0000584
2008-02-20 08:21 Gnomi Note Added: 0000602
2008-02-20 09:27 zesstra Note Added: 0000603
2008-04-01 05:18 zesstra File Added: pcre_limit_v2.diff
2008-04-01 06:01 zesstra Note Added: 0000607
2008-05-06 15:49 zesstra Note Added: 0000614
2008-06-30 04:55 zesstra Status new => assigned
2008-06-30 04:55 zesstra Assigned To => zesstra
2008-07-01 04:50 Gnomi Note Added: 0000639
2008-07-07 05:39 zesstra Note Added: 0000673
2008-07-07 17:37 zesstra Status assigned => resolved
2008-07-07 17:37 zesstra Fixed in Version => 3.3.717
2008-07-07 17:37 zesstra Resolution open => fixed
2008-07-07 17:37 zesstra Note Added: 0000674
2010-11-16 10:42 zesstra Source_changeset_attached => ldmud.git master 4c54a26f
2010-11-16 10:42 zesstra Source_changeset_attached => ldmud.git master 57365be6
2018-01-29 19:59 zesstra Source_changeset_attached => ldmud.git master 4c54a26f
2018-01-29 19:59 zesstra Source_changeset_attached => ldmud.git master 57365be6
2018-01-29 22:57 zesstra Source_changeset_attached => ldmud.git master 4c54a26f
2018-01-29 22:57 zesstra Source_changeset_attached => ldmud.git master 57365be6