public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Егор Чиндяскин <[email protected]>
Cc: [email protected]
Subject: Re: Stack overflow issue
Date: Wed, 24 Aug 2022 12:23:38 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

=?UTF-8?B?0JXQs9C+0YAg0KfQuNC90LTRj9GB0LrQuNC9?= <[email protected]> writes:
> Firstly, we decided to test the regex catalog functions and found 6 of them that lack the check_stach_depth() call.

> zaptreesubs
> markst
> next
> nfatree
> numst
> repeat

I took a closer look at these.  I think the markst, numst, and nfatree
cases are non-issues.  They are recursing on a subre tree that was just
built by parse(), so parse() must have successfully recursed the same
number of levels.  parse() surely has a larger stack frame, and it
does have a stack overflow guard (in subre()), so it would have failed
cleanly before making a data structure that could be hazardous here.
Also, having markst error out would be problematic for the reasons
discussed in its comment, so I'm disinclined to try to add checks
that have no use.

BTW, I wonder why your test didn't notice freesubre()?  But the
same analysis applies there, as does the concern that we can't
just error out.

Likewise, zaptreesubs() can't recurse more levels than cdissect() did,
and that has a stack check, so I'm not very excited about adding
another one there.

I believe that repeat() is a non-issue because (a) the number of
recursion levels in it is limited by DUPMAX, which is generally going
to be 255, or at least not enormous, and (b) it will recurse at most
once before calling dupnfa(), which contains stack checks.

I think next() is a legit issue, although your example doesn't crash
for me.  I suppose that's because my compiler turned the tail recursion
into a loop, and I suggest that we fix it by doing that manually.
(Richard's proposed fix is wrong anyway: we can't just throw elog(ERROR)
in the regex code without creating memory leaks.)

			regards, tom lane

view thread (15+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Stack overflow issue
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox