Re: Improve pg_re_throw: check if sigjmp_buf is valid and report error

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> 于2024年8月20日周二 11:44写道：

> Xiaoran Wang <fanfuxiaoran(at)gmail(dot)com> writes:
> >> Yeah, that's the big problem. I don't have any confidence at all
> >> that this would detect misuse. It'd require that the old stack
> >> frame gets overwritten, which might not happen for a long time,
> >> and it'd require that somebody eventually do a longjmp, which again
> >> might not happen for a long time --- and when those did happen, the
> >> error would be detected in someplace far away from the actual bug,
> >> with little evidence remaining to help you localize it.
>
> > Exactly, it cannot tell you which PG_TRY left the invalid sigjmp_buf,
> > but to implement that is easy I think, recording the line num maybe.
>
> I don't think you get to assume that the canary word gets overwritten
> but debug data a few bytes away survives.
>

We can have a global var like 'PG_exception_debug' to save the debug info,
not saved in the local stack frame.

1. Before PG_TRY, we can save the debug info as 'save_debug_info', just
like
'_save_exception_stack', but not a pointer, memory copy the info into the
'_save_debug_info'.
2. In PG_TRY, set the new debug info into the global var
'PG_exception_debug'
3. And in PG_CATCH and PG_END_TRY, we can restore it.
So that the debug info will not be overwritten.

> It doesn't seem guaranteed that the magic number will get
> overwritten if you do something wrong;

That's my concern too. How about besides checking if the
'PG_exception_stack->magic'
is overwritten, also compare the address of 'PG_exception_stack->buf' and
current
stack top address? if the address of 'PG_exception_stack->buf' is lower
than current
stack top address, it must be invalid, otherwise , it must be overwritten.
Just like below

int stack_top;
if (PG_exception_stack->magic <= &stack_top || PG_exception_stack->magic !=
PG_exception_magic)
ereport(FATAL,
(errmsg("Invalid sigjum_buf, code in PG_TRY cannot contain"
" any non local control flow other than
ereport")));

I'm not sure if this can work, any thoughts?

> regards, tom lane
>

--
Best regards !
Xiaoran Wang