Re: Improve pg_re_throw: check if sigjmp_buf is valid and report error - Mailing list pgsql-hackers

From Xiaoran Wang
Subject Re: Improve pg_re_throw: check if sigjmp_buf is valid and report error
Date
Msg-id CAGjhLkORbtLFD5_8-d8HhPBbXY+A+dS8Qq6F8Dt1FP9zUS8ycg@mail.gmail.com
Whole thread Raw
In response to Re: Improve pg_re_throw: check if sigjmp_buf is valid and report error  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers


Tom Lane <tgl@sss.pgh.pa.us> 于2024年8月20日周二 11:44写道:
Xiaoran Wang <fanfuxiaoran@gmail.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

pgsql-hackers by date:

Previous
From: Kirill Reshke
Date:
Subject: Re: Incremental View Maintenance, take 2
Next
From: Heikki Linnakangas
Date:
Subject: Re: Remaining reference to _PG_fini() in ldap_password_func