GNU bug report logs - #51710
[PATCH] pcre: avoid overflow in PCRE JIT stack resizing

Previous Next

Package: grep;

Reported by: Carlo Marcelo Arenas Belón <carenas <at> gmail.com>

Date: Tue, 9 Nov 2021 08:41:01 UTC

Severity: normal

Tags: patch

Done: Paul Eggert <eggert <at> cs.ucla.edu>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 51710 in the body.
You can then email your comments to 51710 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-grep <at> gnu.org:
bug#51710; Package grep. (Tue, 09 Nov 2021 08:41:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Carlo Marcelo Arenas Belón <carenas <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-grep <at> gnu.org. (Tue, 09 Nov 2021 08:41:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Carlo Marcelo Arenas Belón <carenas <at> gmail.com>
To: bug-grep <at> gnu.org
Cc: Carlo Marcelo Arenas Belón <carenas <at> gmail.com>
Subject: [PATCH] pcre: avoid overflow in PCRE JIT stack resizing
Date: Tue,  9 Nov 2021 00:33:09 -0800
fbc60d4 (Grow the JIT stack if it becomes exhausted, 2015-02-10), add
support to grep for recovering from a JIT stack exhaustion problem,
by creating and using increasingly larger stacks.

The underlying problem might seem to have been generated by a PCRE bug
that is no longer reproducible, and the code could be simplified to do
a single iteration instead with a theoretical maximum of almost INT_MAX,
but that could be a regression, so instead make sure that the maximum
size requested will always be valid, by avoiding a PCRE internal int
overflow that will then be translated into an UINT_MAX like value by
sljit.

Alternatively, a smaller maximum could be selected as it has been
documented[1] that more than 1MB would be unrealistic.

[1] https://www.pcre.org/original/doc/html/pcrejit.html#SEC8

Signed-off-by: Carlo Marcelo Arenas Belón <carenas <at> gmail.com>
---
 src/pcresearch.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/pcresearch.c b/src/pcresearch.c
index 3bdaee9..c4fb09b 100644
--- a/src/pcresearch.c
+++ b/src/pcresearch.c
@@ -77,6 +77,10 @@ jit_exec (struct pcre_comp *pc, char const *subject, int search_bytes,
         {
           int old_size = pc->jit_stack_size;
           int new_size = pc->jit_stack_size = old_size * 2;
+
+          /* PCRE will round up 8K bytes, so avoid overflow in maximum  */
+          if (INT_MAX - new_size < 8192)
+            new_size = INT_MAX - 8192;
           if (pc->jit_stack)
             pcre_jit_stack_free (pc->jit_stack);
           pc->jit_stack = pcre_jit_stack_alloc (old_size, new_size);
-- 
2.34.0.rc1.349.g8f33748433





Reply sent to Paul Eggert <eggert <at> cs.ucla.edu>:
You have taken responsibility. (Tue, 09 Nov 2021 18:29:01 GMT) Full text and rfc822 format available.

Notification sent to Carlo Marcelo Arenas Belón <carenas <at> gmail.com>:
bug acknowledged by developer. (Tue, 09 Nov 2021 18:29:01 GMT) Full text and rfc822 format available.

Message #10 received at 51710-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Carlo Marcelo Arenas Belón <carenas <at> gmail.com>
Cc: 51710-done <at> debbugs.gnu.org
Subject: Re: bug#51710: [PATCH] pcre: avoid overflow in PCRE JIT stack resizing
Date: Tue, 9 Nov 2021 10:28:07 -0800
[Message part 1 (text/plain, inline)]
Thanks for reporting that. I installed the attached somewhat-simpler patch.

Does PCRE2 have a similar bug? If so, I suppose this should be reflected 
when we merge in the patch for bug#47264.
[0001-grep-work-around-PCRE-bug.patch (text/x-patch, attachment)]

Information forwarded to bug-grep <at> gnu.org:
bug#51710; Package grep. (Tue, 09 Nov 2021 19:24:01 GMT) Full text and rfc822 format available.

Message #13 received at 51710-done <at> debbugs.gnu.org (full text, mbox):

From: Carlo Arenas <carenas <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 51710-done <at> debbugs.gnu.org
Subject: Re: bug#51710: [PATCH] pcre: avoid overflow in PCRE JIT stack resizing
Date: Tue, 9 Nov 2021 11:23:15 -0800
No

PCRE2 uses size_t and it is the same (or similar) not signed type when
passed to sljit, so no Undefined Behaviour or overflow.
We might keep the limit in PCRE2 though, as it should be IMHO far
smaller anyway.

Carlo

Car

On Tue, Nov 9, 2021 at 10:28 AM Paul Eggert <eggert <at> cs.ucla.edu> wrote:
>
> Thanks for reporting that. I installed the attached somewhat-simpler patch.
>
> Does PCRE2 have a similar bug? If so, I suppose this should be reflected
> when we merge in the patch for bug#47264.




Information forwarded to bug-grep <at> gnu.org:
bug#51710; Package grep. (Wed, 10 Nov 2021 00:31:02 GMT) Full text and rfc822 format available.

Message #16 received at 51710-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Carlo Arenas <carenas <at> gmail.com>
Cc: 51710-done <at> debbugs.gnu.org
Subject: Re: bug#51710: [PATCH] pcre: avoid overflow in PCRE JIT stack resizing
Date: Tue, 9 Nov 2021 16:30:28 -0800
On 11/9/21 11:23, Carlo Arenas wrote:
> We might keep the limit in PCRE2 though, as it should be IMHO far
> smaller anyway.

The usual GNU rule is "Avoid arbitrary limits on the length or number of 
any data structure" 
<https://www.gnu.org/prep/standards/html_node/Semantics.html>. That 
being said, if PCRE2 greatly misbehaves with a large stack size then we 
should impose some sort of limit, if only to insulate 'grep' from 
PCRE2's problems.




Information forwarded to bug-grep <at> gnu.org:
bug#51710; Package grep. (Sun, 14 Nov 2021 20:56:01 GMT) Full text and rfc822 format available.

Message #19 received at 51710 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Carlo Arenas <carenas <at> gmail.com>
Cc: 51710 <at> debbugs.gnu.org
Subject: Re: bug#51710: [PATCH] pcre: avoid overflow in PCRE JIT stack resizing
Date: Sun, 14 Nov 2021 12:54:58 -0800
As a result of the recent changes to get grep to use PCRE2 instead of 
old PCRE, the relevant code now looks like this:


 /* STACK_GROWTH_RATE is taken from PCRE's src/pcre2_jit_compile.c.
    Going over the jitstack_max limit could trigger an int
    overflow bug.  */
 int STACK_GROWTH_RATE = 8192;
 idx_t jitstack_max = MIN (IDX_MAX, SIZE_MAX - (STACK_GROWTH_RATE - 1));

 int e = pcre2_match (pc->cre, (PCRE2_SPTR) subject, search_bytes,
                      search_offset, options, pc->data, pc->mcontext);
 if (e == PCRE2_ERROR_JIT_STACKLIMIT
     && pc->jit_stack_size <= jitstack_max / 2)
   ... code that computes pc->git_stack_size * 2 ...

This should avoid integer overflow in both grep and libpcre2, without 
imposing arbitrary limits on what PCRE2 can do. If this more-generous 
limit causes problems please let me know.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 13 Dec 2021 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 96 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.