Re: [HACKERS] [PATCH] avoid buffer underflow in errfinish()

2013-12-02 Thread Robert Haas
On Sat, Nov 30, 2013 at 2:00 PM, Bruce Momjian wrote: > On Wed, Mar 27, 2013 at 08:45:51AM -0400, Robert Haas wrote: >> On Sat, Mar 23, 2013 at 6:38 PM, Xi Wang wrote: >> > CHECK_STACK_DEPTH checks if errordata_stack_depth is negative. >> > Move the dereference of &errordata[errordata_stack_depth

Re: [HACKERS] [PATCH] avoid buffer underflow in errfinish()

2013-11-30 Thread Bruce Momjian
On Wed, Mar 27, 2013 at 08:45:51AM -0400, Robert Haas wrote: > On Sat, Mar 23, 2013 at 6:38 PM, Xi Wang wrote: > > CHECK_STACK_DEPTH checks if errordata_stack_depth is negative. > > Move the dereference of &errordata[errordata_stack_depth] after > > the check to avoid out-of-bounds read. > > This

Re: [HACKERS] [PATCH] avoid buffer underflow in errfinish()

2013-03-27 Thread Xi Wang
On Wed, Mar 27, 2013 at 9:03 AM, Heikki Linnakangas wrote: > Writing it like that suggests that autovac might sometimes be NULL, even if > deadlock_state == DS_BLOCKED_BY_AUTOVACUUM. From your explanation above, I > gather that's not possible (and I think you're right), so the NULL check is > unne

Re: [HACKERS] [PATCH] avoid buffer underflow in errfinish()

2013-03-27 Thread Heikki Linnakangas
On 27.03.2013 14:50, Robert Haas wrote: On Sat, Mar 23, 2013 at 6:45 PM, Xi Wang wrote: A side question: at src/backend/storage/lmgr/proc.c:1150, is there a null pointer deference for `autovac'? I think you mean on line 1142: PGPROC *autovac = GetBlockingAutoVacuumPgproc

Re: [HACKERS] [PATCH] avoid buffer underflow in errfinish()

2013-03-27 Thread Robert Haas
On Sat, Mar 23, 2013 at 6:45 PM, Xi Wang wrote: > A side question: at src/backend/storage/lmgr/proc.c:1150, is there a > null pointer deference for `autovac'? Not really. If the deadlock_state is DS_BLOCKED_BY_AUTOVACUUM, there has to be a blocking autovacuum proc. As in the other case that you

Re: [HACKERS] [PATCH] avoid buffer underflow in errfinish()

2013-03-27 Thread Robert Haas
On Sat, Mar 23, 2013 at 6:38 PM, Xi Wang wrote: > CHECK_STACK_DEPTH checks if errordata_stack_depth is negative. > Move the dereference of &errordata[errordata_stack_depth] after > the check to avoid out-of-bounds read. This seems sensible and I'm inclined to commit it. It's unlikely to matter v

Re: [HACKERS] [PATCH] avoid buffer underflow in errfinish()

2013-03-23 Thread Xi Wang
A side question: at src/backend/storage/lmgr/proc.c:1150, is there a null pointer deference for `autovac'? There is a null pointer check `autovac != NULL', but the pointer is already dereferenced earlier when initializing `autovac_pgxact'. Is this null pointer check redundant, or should we move t

[HACKERS] [PATCH] avoid buffer underflow in errfinish()

2013-03-23 Thread Xi Wang
CHECK_STACK_DEPTH checks if errordata_stack_depth is negative. Move the dereference of &errordata[errordata_stack_depth] after the check to avoid out-of-bounds read. --- src/backend/utils/error/elog.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/error