BUG #17995: Segmentation fault caused by UPDATE statement

PG Bug reporting form <noreply@postgresql.org> writes:

--- Test case ---
create table t1 (pkey int4, c7 float8, c8 text, c9 float8);
insert into t1 (pkey, c7, c8, c9) values (96000, 0.0, '3n@', -79.14);
update t1 set c7 = t1.c9 / t1.c7 where 'a' @@ repeat(t1.c8, t1.pkey);

Hmm, I don't think this is about the UPDATE per se, it's about
not having a stack depth check in TParserGet() :-(

regards, tom lane

A time ago I had a problem with segmentation fault, and it was solved
disabling jit.

On Sat, Jun 24, 2023, 16:52 Tom Lane <tgl@sss.pgh.pa.us> wrote:

Hello,

24.06.2023 22:51, Tom Lane wrote:

Hmm, I don't think this is about the UPDATE per se, it's about
not having a stack depth check in TParserGet() :-(

BTW, there is a commitfest entry to eliminate a bunch of other stack
overflow hazards (may be the fuzzer can find some of them too):
https://commitfest.postgresql.org/43/4239/

(It looks like this issue was not discovered there because TParserGet()
doesn't call itself directly.)

Best regards,
Alexander

On Sun, Jun 25, 2023 at 06:00:00AM +0300, Alexander Lakhin wrote:

BTW, there is a commitfest entry to eliminate a bunch of other stack
overflow hazards (may be the fuzzer can find some of them too):
https://commitfest.postgresql.org/43/4239/

Thanks for the pointer, I'll double-check that. Some of the locations
of stack depth checks proposed involve performance-sensitive code
paths, though, like mcxt.c :/
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Sun, Jun 25, 2023 at 06:00:00AM +0300, Alexander Lakhin wrote:

BTW, there is a commitfest entry to eliminate a bunch of other stack
overflow hazards (may be the fuzzer can find some of them too):
https://commitfest.postgresql.org/43/4239/

Thanks for the pointer, I'll double-check that. Some of the locations
of stack depth checks proposed involve performance-sensitive code
paths, though, like mcxt.c :/

I hadn't looked at the patch yet, but ... mcxt.c? How is that recursive?
Even if there is some path that recurses through that, wouldn't the
check be better placed in a less-hot part of the loop?

regards, tom lane

On Mon, Jun 26, 2023 at 12:27:08AM -0400, Tom Lane wrote:

Michael Paquier <michael@paquier.xyz> writes:

Thanks for the pointer, I'll double-check that. Some of the locations
of stack depth checks proposed involve performance-sensitive code
paths, though, like mcxt.c :/

I hadn't looked at the patch yet, but ... mcxt.c? How is that recursive?
Even if there is some path that recurses through that, wouldn't the
check be better placed in a less-hot part of the loop?

Yeah, that's my impression. I'll switch to the other thread after
looking more in details.
--
Michael

26.06.2023 07:27, Tom Lane wrote:

I hadn't looked at the patch yet, but ... mcxt.c? How is that recursive?
Even if there is some path that recurses through that, wouldn't the
check be better placed in a less-hot part of the loop?

In that thread Egor Chindyaskin showed proofs for stack overflow in all the
functions, that the fixes were proposed for. For example:
#MemoryContextStatsInternal
(n=1000000; printf "BEGIN;"; for ((i=1;i<=$n;i++)); do printf "SAVEPOINT s$i;"; done; printf "SELECT
pg_log_backend_memory_contexts(pg_backend_pid())") | psql >/dev/null

Though maybe for some of those functions less-hot places could be found to
check the stack.

Best regards,
Alexander